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

Handle mc and fr questions as ref #774

Merged
merged 8 commits into from
Feb 3, 2025

Conversation

johnarban
Copy link
Collaborator

This PR refactors the Multiple Choice and Free Response questions to be nested dictionaries that are updated using Refs. This PR also makes sure the stage they belong to is a part of the question so they can be distinguished in the dashboard.

The general sequence is

free_responses = Ref(local_state.fields.free_responses).value.copy()['responses']
// do something ot the free responses
Ref(local_state.fields.free_responses).set({'responses': free_responses})

The .copy() seems necessary to prevent sharing modifications to free_response or 'mc_scoring` with other users. the mc and fr questions no longer have Pydantic models associated with them.

@johnarban johnarban requested a review from nmearl January 27, 2025 23:05
Copy link
Collaborator

@nmearl nmearl left a comment

Choose a reason for hiding this comment

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

This is looking good to me. But I haven't tried without @Carifio24's quick fix. Can you give this a shot after removing Jon's changes and see if you fix pre-filled FR or MC questions in two different app instances?

@johnarban
Copy link
Collaborator Author

This is looking good to me. But I haven't tried without @Carifio24's quick fix. Can you give this a shot after removing Jon's changes and see if you fix pre-filled FR or MC questions in two different app instances?

@nmearl , I ran it with the commits on cosmicds/cosmicds#361 removed. The free-response and mulitple choice questions were not shared between sessions for two different users run on a single solara server instance

Copy link
Collaborator

@nmearl nmearl left a comment

Choose a reason for hiding this comment

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

Excellent -- looks good to me then!

@patudom
Copy link
Contributor

patudom commented Feb 3, 2025

Great, thanks @johnarban & @nmearl. Do we still need cosmicds/cosmicds#361 or do we want to undo those?

@patudom patudom merged commit 9b58428 into cosmicds:main Feb 3, 2025
@patudom
Copy link
Contributor

patudom commented Feb 3, 2025

And do we want to bring this over to the demo branch, or should we leave that alone since we know it works?

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.

3 participants