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

CourseViewOption record with defaults is not created when new Course record is created in Admin #668

Closed
ssciolla opened this issue Aug 19, 2019 · 4 comments
Assignees
Labels
🪳 bug Something isn't working

Comments

@ssciolla
Copy link
Contributor

ssciolla commented Aug 19, 2019

Thank you for contributing to this project!

  • Make sure to search the issues for duplicates first!

Expected behavior (A clear and concise description of what you expected to happen) :

When a new Course is created in the Django admin interface, a new CourseViewOption record should also be created, regardless of what boxes are checked for the three view inline settings ("Show Resources Accessed View", "Show Assignment Planning View," and "Show Grade Distribution View").

Describe the bug (Tell us what happens instead of the expected behavior) :

When a new Course is created through the Admin with the default options selected for the CourseViewOption inline integer fields, a new CourseViewOption record is NOT created. If you uncheck any of the boxes (diverging from the default), a CourseViewOption record is created.

Steps to Reproduce :

  1. In the Django admin interface, click the "Add Course" button.
  2. Fill out the form without modifying the three CourseViewOption checkboxes.
  3. Look at the Courses Dashboard, where there will be no views listed under "COURSE VIEW OPTION(S)" for the new course. Alternatively, look at the course_view_option table in the database, noting the absence of a new record.

Screenshots/Video (If possible, add media to help explain your problem) :

Additional context (Add any other context about the problem here) :

This bug was previously documented and fixed by @jonespm (see PR #452). I believe it was reintroduced when I removed the editable=false option from the id field in Course in models.py as part of PR #652 (thanks to @pushyamig for alerting me to the issue!). This bug could be fixed by re-adding the editable=false option. However, this problem and Issue #613 might be fixed by adding AutoField primary keys to both the Course and CourseViewOption models and making the table relationship dependent on those ids rather than field values we may want to change. I will investigate the latter option and report back. If that is a dead end, I will re-add the editable=false option.

@ssciolla ssciolla added the 🪳 bug Something isn't working label Aug 19, 2019
@ssciolla ssciolla added this to To do in MyLA-Fall-2019.02.01 via automation Aug 19, 2019
@project-bot project-bot bot added this to To do in MyLA-Default-Project Aug 19, 2019
@ssciolla ssciolla self-assigned this Aug 19, 2019
@jonespm
Copy link
Member

jonespm commented Aug 21, 2019

It should still have editable=false.

The save_model override of CourseAdmin is intended for creating and saving the id based on the canvas_id. This isn't a field the user should need to enter. It's possible this isn't executing correctly if this isn't in there.

    def save_model(self, request, obj, form, change):
        obj.id = canvas_id_to_incremented_id(obj.canvas_id)
        return super(CourseAdmin, self).save_model(request, obj, form, change)

@ssciolla
Copy link
Contributor Author

I've been experimenting with using readonly_fields in the CourseAdmin rather than using editable=false. It's nice that the field still shows up in the interface, without being editable. The editable option does seem to be more global, which could be an advantage. Both seem to solve this issue, making the AlwaysChangedModelForm work as expected.

@ssciolla
Copy link
Contributor Author

This has been resolved with PR #666; course_id was made read-only in the admin interface for Course.

MyLA-Fall-2019.02.01 automation moved this from To do to Review/QA Aug 27, 2019
MyLA-Default-Project automation moved this from To do to Review/QA Aug 27, 2019
@jennlove-um
Copy link
Contributor

Testing passed in #666. I added a course and the course_id was read only and populated when the chron job ran.

@jennlove-um jennlove-um moved this from Review/QA to Done in MyLA-Fall-2019.02.01 Sep 18, 2019
@jennlove-um jennlove-um removed this from Review/QA in MyLA-Default-Project Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working
Projects
No open projects
Development

No branches or pull requests

3 participants