feat(duplication): sort destination instances #8207
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements sorting functionality for destination instances in the course duplication feature. The primary goal is to ensure the current instance appears first in the destination instance dropdown, followed by other instances sorted alphabetically.
Changes:
- Backend sorts destination instances with current instance first, then alphabetically by name
- Frontend store assigns weight to instances based on backend sort order
- Frontend dropdown sorts instances by weight for display
- Added UI button to quickly select current instance
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/views/course/object_duplications/new.json.jbuilder | Adds sorting logic to place current tenant's instance first, followed by others alphabetically |
| client/app/bundles/course/duplication/store.js | Removes unused import and adds weight property to instances based on sort order |
| client/app/bundles/course/duplication/pages/Duplication/DestinationCourseSelector/NewCourseForm.jsx | Passes currentInstanceId and setValue props to InstanceDropdown component |
| client/app/bundles/course/duplication/pages/Duplication/DestinationCourseSelector/InstanceDropdown.tsx | Implements sorting by weight, adds button to select current instance, and updates UI layout |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| className={`${ | ||
| currentInstanceId === field.value ? 'text-blue-500' : null | ||
| }`} |
There was a problem hiding this comment.
The className expression ${currentInstanceId === field.value ? 'text-blue-500' : null} will render the string "null" when the condition is false. This should use an empty string instead of null to avoid rendering "null" as a class name.
| className={`${ | |
| currentInstanceId === field.value ? 'text-blue-500' : null | |
| }`} | |
| className={ | |
| currentInstanceId === field.value ? 'text-blue-500' : '' | |
| } |
| disabled={ | ||
| disabled || | ||
| !metadata.canDuplicateToAnotherInstance || | ||
| instances.length > 1 |
There was a problem hiding this comment.
The condition instances.length > 1 is incorrect. The instances variable is now an object (created with Object.fromEntries in the store), not an array, so .length will be undefined. This should be Object.keys(instances).length > 1 to check if there is more than one instance.
| instances.length > 1 | |
| Object.keys(instances).length > 1 |
No description provided.