Skip to content

Commit

Permalink
Optimize the Instructors for Course Report
Browse files Browse the repository at this point in the history
Because of the joins required this report performed very poorly using
the API as the number of sessions and offerings in a course grew.
Instead we can use our GraphQL API to get the data we want from the
course down and significantly improve the performance.

I didn't DRY this up, we probably need to replace the reporting service
for download and re-use the data fetching in the subject components, but
that is for another day.
  • Loading branch information
jrjohnson committed Aug 16, 2024
1 parent 05a7253 commit 54aa10e
Show file tree
Hide file tree
Showing 4 changed files with 239 additions and 45 deletions.
49 changes: 42 additions & 7 deletions packages/frontend/app/components/reports/subject/instructor.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { cached } from '@glimmer/tracking';
import { service } from '@ember/service';
import { pluralize } from 'ember-inflector';
import { camelize, capitalize } from '@ember/string';
import { uniqueById } from 'ilios-common/utils/array-helpers';

export default class ReportsSubjectInstructorComponent extends Component {
@service graphql;
Expand Down Expand Up @@ -50,13 +51,7 @@ export default class ReportsSubjectInstructorComponent extends Component {
}
if (prepositionalObject && prepositionalObjectTableRowId) {
let what = pluralize(camelize(prepositionalObject));
const specialInstructed = [
'learningMaterials',
'sessionTypes',
'courses',
'sessions',
'academicYears',
];
const specialInstructed = ['learningMaterials', 'sessionTypes', 'sessions', 'academicYears'];
if (specialInstructed.includes(what)) {
what = 'instructed' + capitalize(what);
}
Expand All @@ -66,11 +61,51 @@ export default class ReportsSubjectInstructorComponent extends Component {
return rhett;
}

async getResultsForCourse(courseId) {
const userInfo = '{ id firstName middleName lastName displayName }';
const block = `instructorGroups { users ${userInfo}}instructors ${userInfo}`;
const results = await this.graphql.find(
'courses',
[`id: ${courseId}`],
`sessions {
ilmSession { ${block} }
offerings { ${block} }
}`,
);

if (!results.data.courses.length) {
return [];
}

const users = results.data.courses[0].sessions.reduce((acc, session) => {
if (session.ilmSession) {
acc.push(
...session.ilmSession.instructors,
...session.ilmSession.instructorGroups.flatMap((group) => group.users),
);
}
session.offerings.forEach((offering) => {
acc.push(
...offering.instructors,
...offering.instructorGroups.flatMap((group) => group.users),
);
});

return acc;
}, []);

return uniqueById(users);
}

async getReportResults(subject, prepositionalObject, prepositionalObjectTableRowId, school) {
if (subject !== 'instructor') {
throw new Error(`Report for ${subject} sent to ReportsSubjectInstructorComponent`);
}

if (prepositionalObject === 'course') {
return this.getResultsForCourse(prepositionalObjectTableRowId);
}

const filters = await this.getGraphQLFilters(
prepositionalObject,
prepositionalObjectTableRowId,
Expand Down
81 changes: 61 additions & 20 deletions packages/frontend/app/services/reporting.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import Service, { service } from '@ember/service';
import { pluralize } from 'ember-inflector';
import { camelize, capitalize, dasherize } from '@ember/string';
import striptags from 'striptags';
import { mapBy, sortBy } from 'ilios-common/utils/array-helpers';
import { mapBy, sortBy, uniqueById } from 'ilios-common/utils/array-helpers';
import { map } from 'rsvp';

const subjectTranslations = {
Expand Down Expand Up @@ -160,27 +160,68 @@ export default class ReportingService extends Service {
].concat(mappedResults);
}

async getInstructorsArrayResultsForCourse(courseId) {
const userInfo = '{ id firstName middleName lastName displayName }';
const block = `instructorGroups { users ${userInfo}}instructors ${userInfo}`;
const results = await this.graphql.find(
'courses',
[`id: ${courseId}`],
`sessions {
ilmSession { ${block} }
offerings { ${block} }
}`,
);

if (!results.data.courses.length) {
return [];
}

const users = results.data.courses[0].sessions.reduce((acc, session) => {
if (session.ilmSession) {
acc.push(
...session.ilmSession.instructors,
...session.ilmSession.instructorGroups.flatMap((group) => group.users),
);
}
session.offerings.forEach((offering) => {
acc.push(
...offering.instructors,
...offering.instructorGroups.flatMap((group) => group.users),
);
});

return acc;
}, []);

return uniqueById(users);
}

async instructorsArrayResults(report) {
const filters = await this.#getFilters(report);
const graphqlFilters = filters.map((filter) => {
const specialInstructed = [
'learningMaterials',
'sessionTypes',
'courses',
'sessions',
'academicYears',
];
specialInstructed.forEach((special) => {
if (filter.includes(special)) {
const cap = capitalize(special);
filter = filter.replace(special, `instructed${cap}`);
}
let users;
if (report.prepositionalObject === 'course') {
users = await this.getInstructorsArrayResultsForCourse(report.prepositionalObjectTableRowId);
} else {
const filters = await this.#getFilters(report);
const graphqlFilters = filters.map((filter) => {
const specialInstructed = [
'learningMaterials',
'sessionTypes',
'sessions',
'academicYears',
];
specialInstructed.forEach((special) => {
if (filter.includes(special)) {
const cap = capitalize(special);
filter = filter.replace(special, `instructed${cap}`);
}
});
return filter;
});
return filter;
});
const attributes = ['id', 'firstName', 'middleName', 'lastName', 'displayName'];
const result = await this.graphql.find('users', graphqlFilters, attributes.join(','));
const names = result.data.users
const attributes = ['id', 'firstName', 'middleName', 'lastName', 'displayName'];
users = await this.graphql.find('users', graphqlFilters, attributes.join(','));
}

const names = users
.map(({ firstName, middleName, lastName, displayName }) => {
if (displayName) {
return displayName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ module('Integration | Component | reports/subject/instructor', function (hooks)
const { query } = JSON.parse(requestBody);
assert.strictEqual(
query,
'query { users(instructedCourses: [13]) { firstName,middleName,lastName,displayName } }',
`query { courses(id: 13) { sessions {
ilmSession { instructorGroups { users { id firstName middleName lastName displayName }}instructors { id firstName middleName lastName displayName } }
offerings { instructorGroups { users { id firstName middleName lastName displayName }}instructors { id firstName middleName lastName displayName } }
} } }`,
);
return responseData;
});
Expand Down Expand Up @@ -135,4 +138,27 @@ module('Integration | Component | reports/subject/instructor', function (hooks)
@prepositionalObjectTableRowId={{this.report.prepositionalObjectTableRowId}}
/>`);
});

test('filter by session types', async function (assert) {
assert.expect(1);
this.server.post('api/graphql', function (schema, { requestBody }) {
const { query } = JSON.parse(requestBody);
assert.strictEqual(
query,
'query { users(instructedSessionTypes: [4]) { firstName,middleName,lastName,displayName } }',
);
return responseData;
});
const { id } = this.server.create('report', {
subject: 'instructor',
prepositionalObject: 'session type',
prepositionalObjectTableRowId: 4,
});
this.set('report', await this.owner.lookup('service:store').findRecord('report', id));
await render(hbs`<Reports::Subject::Instructor
@subject={{this.report.subject}}
@prepositionalObject={{this.report.prepositionalObject}}
@prepositionalObjectTableRowId={{this.report.prepositionalObjectTableRowId}}
/>`);
});
});
126 changes: 109 additions & 17 deletions packages/frontend/tests/unit/services/reporting-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -354,41 +354,133 @@ module('Unit | Service | reporting', function (hooks) {

test('getArrayResults() for instructor report', async function (assert) {
assert.expect(2);
const school = this.server.create('school', { title: 'School of Schools' });
const course = this.server.create('course', { id: 42, school });
const session = this.server.create('session', { course });
const ilmSession = this.server.create('ilmSession', { session });
const report = this.server.create('report', {
subject: 'instructor',
school,
prepositionalObject: 'course',
prepositionalObjectTableRowId: 42,
});
const instructors = this.server.createList('user', 2, { instructorIlmSessions: [ilmSession] });
this.server.post('api/graphql', function (schema, { requestBody }) {
const { query } = JSON.parse(requestBody);
assert.strictEqual(
query,
'query { users(schools: [1], instructedCourses: [42]) { id,firstName,middleName,lastName,displayName } }',
`query { courses(id: 42) { sessions {
ilmSession { instructorGroups { users { id firstName middleName lastName displayName }}instructors { id firstName middleName lastName displayName } }
offerings { instructorGroups { users { id firstName middleName lastName displayName }}instructors { id firstName middleName lastName displayName } }
} } }`,
);
return {
data: {
users: instructors
.reverse()
.map(({ id, firstName, middleName, lastName, displayName }) => ({
id,
firstName,
middleName,
lastName,
displayName,
})),
courses: [
{
sessions: [
{
ilmSession: {
instructors: [
{
id: 1,
firstName: 'F',
middleName: '1',
lastName: 'L',
displayName: '',
},
],
instructorGroups: [
{
users: [
{
id: 2,
firstName: 'F',
middleName: '2',
lastName: 'L',
displayName: '',
},
],
},
],
},
offerings: [
{
instructors: [
{
id: 3,
firstName: 'F',
middleName: '',
lastName: 'L',
displayName: 'o1i',
},
{
id: 2,
firstName: 'F',
middleName: '2',
lastName: 'L',
displayName: '',
},
],
instructorGroups: [
{
users: [
{
id: 4,
firstName: 'F',
middleName: '',
lastName: 'L',
displayName: 'o1ig',
},
],
},
],
},
{
instructors: [
{
id: 5,
firstName: 'F',
middleName: '',
lastName: 'L',
displayName: 'o2i',
},
],
instructorGroups: [
{
users: [
{
id: 6,
firstName: 'F',
middleName: '',
lastName: 'L',
displayName: 'o2ig',
},
{
id: 2,
firstName: 'F',
middleName: '2',
lastName: 'L',
displayName: '',
},
],
},
],
},
],
},
],
},
],
},
};
});

const store = this.owner.lookup('service:store');
const reportModel = await store.findRecord('report', report.id);
const props = await this.service.getArrayResults(reportModel, 1999);
assert.deepEqual(props, [['Instructors'], ['0 guy M. Mc0son'], ['1 guy M. Mc1son']]);
assert.deepEqual(props, [
['Instructors'],
['F 1. L'],
['F 2. L'],
['o1i'],
['o1ig'],
['o2i'],
['o2ig'],
]);
});
});

0 comments on commit 54aa10e

Please sign in to comment.