-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sub-segments of used segment will show as used in segments list #1057
sub-segments of used segment will show as used in segments list #1057
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One suggestion
@@ -110,6 +110,24 @@ export class SegmentService { | |||
}); | |||
} | |||
|
|||
segmentsData.forEach((segment) => { | |||
segmentsUsedLockedList.forEach((usedSegment) => { | |||
if (segment.id === usedSegment) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can replace this third-level loop with something like: segmentsUsedLockedList.push(...segment.subSegments.map(subSegment => subSegment.id))
and also below for the unlocked list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the second level loop can be replaced with an if condition
if(segmentUsedList.includes(segment.id))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
return { ...segment, status: SEGMENT_STATUS.USED }; // TODO change to locked | ||
} else if (segmentsUsedUnlockedList.find((segmentId) => segmentId === segment.id)) { | ||
return { ...segment, status: SEGMENT_STATUS.USED }; // TODO change to unlocked | ||
} else if (segmentsUsedList.find((segmentId) => segmentId === segment.id)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use includes
over here also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
@@ -85,38 +85,35 @@ export class SegmentService { | |||
const allExperimentSegmentsInclusion = await this.getExperimentSegmenInclusionData(); | |||
const allExperimentSegmentsExclusion = await this.getExperimentSegmenExclusionData(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These three sequential awaits should be executed parallelly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added some comments
https://github.com/CarnegieLearningWeb/UpGrade into bugfix/subsegments-of-used-segment-not-showed-as-used
This PR will resolve the issue of sub-segments of used segments not showing as used.