-
Notifications
You must be signed in to change notification settings - Fork 716
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
Remove resource pool from exam serializer, don't send in API call #12200
Remove resource pool from exam serializer, don't send in API call #12200
Conversation
const resourcePoolAsIds = get(section).resource_pool.map(content => content.id); | ||
section.resource_pool = resourcePoolAsIds; | ||
const questionSourcesWithoutResourcePool = get(allSections).map(section => { | ||
delete section.resource_pool; |
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.
It might be better to clone the object first:
const sectionToSave = {
...section,
};
delete sectionToSave.resource_pool;
return sectionToSave;
This will prevent unintended mutation of the composable state.
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.
(note, this applied to the previous code as well, it just only occurred to me now, as we are considering allowing editing after saving)
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.
Thanks for that - pushed an update
Build Artifacts
|
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.
Looks good to me - does this need any manual confirmation?
If it does, please add the reviewer guidance or a test case description, it's not immediately clear from the PR summary 😅 |
@rtibbles @radinamatic I think this is okay without any additional testing - I've successfully made and taken a quiz w/ these changes applied & tests pass so should be okay on passing code review |
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.
Code review looks good.
Summary
The
resource_pool
is not used when reading exam data back from the database and can be derived from the list of questions in each section by theirexercise_id
.This removes it from the serializer and ensures it doesn't get send to the API when saving a quiz.