Skip to content
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

Fix reordering of lessons via drag-and-drop on the script edit gui page #37440

Merged
merged 12 commits into from Oct 27, 2020

Conversation

davidsbailey
Copy link
Member

@davidsbailey davidsbailey commented Oct 25, 2020

Fixes the issue where reordering lessons doesn't work. the root cause is that the lesson ordering code expects position to be relative to the lesson group (parent), but the lesson position is currently set relative to the unit (grandparent).

Specific changes:

  • set lesson position relative to lesson group, not unit
  • remove unused relativePosition field
  • remove unused moveLesson redux action
  • eliminate unnecessary references to lesson position, now that the first lesson in each lesson group has position 1

Testing story

  • added test cases for reorderLesson redux action.
  • manually verified that various dragging scenarios work when multiple lesson groups are present.

Reviewer Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@davidsbailey davidsbailey marked this pull request as ready for review October 26, 2020 00:41
@davidsbailey davidsbailey changed the title Reorder lessons Fix reordering of lessons via drag-and-drop on the script edit gui page Oct 26, 2020
@dmcavoy
Copy link
Contributor

dmcavoy commented Oct 26, 2020

Did you check that script saving works?

@davidsbailey
Copy link
Member Author

Did you check that script saving works?

Yep! I think it should be possible to add a UI test for this soon.

Copy link
Contributor

@dmcavoy dmcavoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@davidsbailey davidsbailey merged commit cb3e9d5 into staging Oct 27, 2020
@davidsbailey davidsbailey deleted the reorder-lessons branch October 27, 2020 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants