fix: update advanced module list not working #2189
Merged
Conversation
This file contains hidden or 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
Backend was still expecting `{'advanced_modules', {'value': ['poll', 'problem-builder', 'h5pxblock']}}` but without this change, it was receiving `{'advancedModules', ['poll', 'problem-builder', 'h5pxblock']}`
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2189 +/- ##
=========================================
Coverage ? 94.08%
=========================================
Files ? 1164
Lines ? 24481
Branches ? 5305
=========================================
Hits ? 23033
Misses ? 1380
Partials ? 68 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@jignaciopm @arbrandes can you please take a look? Why is this so complicated :p |
jignaciopm
approved these changes
Jun 18, 2025
There was a problem hiding this comment.
@bradenmacdonald It was wrong to remove that in PR#1581... This change is correct. When this PR is merged, I'll apply the backport in my PR #2087
bradenmacdonald
approved these changes
Jun 18, 2025
There was a problem hiding this comment.
OK then good with me. But... shouldn't we have some test that catches this error? I think the test is mocked at too high of level, so it didn't catch this. The test should only mock the raw HTTP response coming from the API and then assert on the raw API response send back to the server after the settings are edited, and that would ensure bugs like this don't happen.
jignaciopm
pushed a commit
to eduNEXT/frontend-app-course-authoring
that referenced
this pull request
Jun 19, 2025
Backend was still expecting `{'advanced_modules', {'value': ['poll', 'problem-builder', 'h5pxblock']}}` but without this change, it was receiving `{'advancedModules', ['poll', 'problem-builder', 'h5pxblock']}`
Follow up to openedx#1581
Co-authored-by: Muhammad Faraz Maqsood <[email protected]>
bradenmacdonald
pushed a commit
that referenced
this pull request
Jun 19, 2025
…rt) (#2087) * fix: advanced-settings api should not camel-case return value (#1581) * fix: update advanced module list not working (#2189) Backend was still expecting `{'advanced_modules', {'value': ['poll', 'problem-builder', 'h5pxblock']}}` but without this change, it was receiving `{'advancedModules', ['poll', 'problem-builder', 'h5pxblock']}` Follow up to #1581 --------- Co-authored-by: Muhammad Faraz Maqsood <[email protected]>
luisfelipec95
pushed a commit
to eduNEXT/frontend-app-course-authoring
that referenced
this pull request
Jul 4, 2025
…rt) (openedx#2087) * fix: advanced-settings api should not camel-case return value (openedx#1581) * fix: update advanced module list not working (openedx#2189) Backend was still expecting `{'advanced_modules', {'value': ['poll', 'problem-builder', 'h5pxblock']}}` but without this change, it was receiving `{'advancedModules', ['poll', 'problem-builder', 'h5pxblock']}` Follow up to openedx#1581 --------- Co-authored-by: Muhammad Faraz Maqsood <[email protected]>
luisfelipec95
pushed a commit
to eduNEXT/frontend-app-course-authoring
that referenced
this pull request
Jul 7, 2025
…rt) (openedx#2087) * fix: advanced-settings api should not camel-case return value (openedx#1581) * fix: update advanced module list not working (openedx#2189) Backend was still expecting `{'advanced_modules', {'value': ['poll', 'problem-builder', 'h5pxblock']}}` but without this change, it was receiving `{'advancedModules', ['poll', 'problem-builder', 'h5pxblock']}` Follow up to openedx#1581 --------- Co-authored-by: Muhammad Faraz Maqsood <[email protected]>
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.
Ticket: TNL-12010
Backend was expecting
{'advanced_modules', {'value': ['poll', 'problem-builder', 'h5pxblock']}}but after this PR merged, it was receiving
{'advancedModules', ['poll', 'problem-builder', 'h5pxblock']}It's better to send snake case settings from frontend instead of changing/fixing the function at backend here.
If we change line 262 from
val = model['value']toval = model, it will still fail on next lineif hasattr(block, key) and getattr(block, key) != val:ashasattr(block, 'advancedModules')is incorrect. It needsadvancedModulesas key.Before:
After:
