Remove unneeded ilios common slices #8116
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
michaelchadwick
requested review from
jrjohnson and
stopfstedt
as code owners
jrjohnson
requested changes
Sep 9, 2024
| return findById(objectives, objective.id); | ||
| }); | ||
| this.isManagingParents = true; | ||
| }); | ||
|
|
||
| manageDescriptors = dropTask(async () => { | ||
| const meshDescriptors = await this.args.courseObjective.meshDescriptors; | ||
| this.descriptorsBuffer = meshDescriptors.slice(); |
There was a problem hiding this comment.
The entire purpose of this interim variable seems to be the slice. This might be the only time we notice this for a while so please replace with a direct assignment eg this.descriptorsBuffer = await this.args.courseObjective.meshDescriptors;
| @@ -138,7 +138,7 @@ export default class LearningMaterialManagerComponent extends Component { | |||
| this.endDate = learningMaterial.endDate; | |||
|
|
|||
| const meshDescriptors = await learningMaterial.get('meshDescriptors'); | |||
| @@ -59,20 +59,20 @@ export default class SessionObjectiveListItemComponent extends Component { | |||
|
|
|||
| manageParents = dropTask(async () => { | |||
| const parents = await this.args.sessionObjective.courseObjectives; | |||
| this.parentsBuffer = parents.slice(); | |||
| this.parentsBuffer = parents; | |||
| this.isManagingParents = true; | ||
| }); | ||
|
|
||
| manageDescriptors = dropTask(async () => { | ||
| const meshDescriptors = await this.args.sessionObjective.meshDescriptors; | ||
| this.descriptorsBuffer = meshDescriptors.slice(); | ||
| this.descriptorsBuffer = meshDescriptors; |
| this.isManagingDescriptors = true; | ||
| }); | ||
|
|
||
| manageTerms = dropTask(async (vocabulary) => { | ||
| this.selectedVocabulary = vocabulary; | ||
| const terms = await this.args.sessionObjective.terms; | ||
| this.termsBuffer = terms.slice(); | ||
| this.termsBuffer = terms; |
| @@ -87,7 +87,7 @@ export default class SessionsGrid extends Component { | |||
| const offerings = await session.offerings; | |||
| const learnerGroups = await map(offerings, async (offering) => { | |||
| const learnerGroups = await offering.learnerGroups; | |||
| return learnerGroups.slice(); | |||
| return learnerGroups; | |||
| @@ -15,7 +15,7 @@ export default class CourseVisualizeInstructorRoute extends Route { | |||
| } | |||
|
|
|||
| async afterModel({ course }) { | |||
| const sessions = (await course.sessions).slice(); | |||
| const sessions = await course.sessions; | |||
| return await all([course.school, map(sessions, (s) => s.sessionType)]); | |||
There was a problem hiding this comment.
Interesting thing here. It looks like RSVP.map will resolve a promise that is passed as the first argument. Really for discussion, but would it be cleaner to do this without the interim variable as just map(course.sessions, fn)?
There was a problem hiding this comment.
Seems like a cromulent change. Probably need to do this across the site, but I'm checking to see if the change kills any tests now.
jrjohnson
requested changes
Sep 9, 2024
| const meshDescriptors = await learningMaterial.get('meshDescriptors'); | ||
| this.terms = meshDescriptors.slice(); | ||
|
|
||
| this.terms = await learningMaterial.get('meshDescriptors'); |
There was a problem hiding this comment.
I should have noticed this the first time, sorry, this should be
Suggested change
| this.terms = await learningMaterial.get('meshDescriptors'); | |
| this.terms = await learningMaterial.meshDescriptors; |
and remove the old school Ember pattern of using get here.
| const sessions = (await course.sessions).slice(); | ||
| return await all([course.school, map(sessions, (s) => s.sessionType)]); | ||
| // const sessions = await course.sessions; | ||
| console.log('CourseVisualizeInstructorRoute'); |
michaelchadwick
force-pushed
the
remove-unneeded-ilios-common-slices
branch
from
4d12a8f
to
3bb1258
Compare
jrjohnson
approved these changes
Sep 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References ilios/ilios#5660
Went through the
packages/ilios-commondirectory and removed a bunch ofslice()s. Left some I found because they had a.sort()or[1]or similar access thing that I know has thrown off our tests in the past.