Read published_state and pilot_experiment from unit_group if script is in one

[bethanyaconnor]

Step 1 of setting up per-unit-in-course published_state.

It turns out that there are some scripts that have a different published_state from their unit group (I believe this is needed for alternative scripts?). New steps:

1. (This PR) Add get_published_state and get_pilot_experiment methods to scripts, which reads from the script first then the unit group (if there is one).
   • Scripts that differ from their unit group:

[levelbuilder] dashboard > Script.all.select {|s| s.unit_group && s.published_state != s.unit_group.published_state}.map(&:name)
=> ["csp-post-survey", "csp3-research-mxghyt", "csp3-a", "csd-post-survey-2018", "csp-post-survey-2018", "time4cs-experiment-unit-2", "time4cs-control-unit-1"]
[levelbuilder] dashboard > Script.all.select {|s| s.unit_group && s.pilot_experiment != s.unit_group.pilot_experiment}.map(&:name)
=> ["time4cs-experiment-unit-2", "time4cs-control-unit-1"]

2. (Allow the script published_state to be null #41616) Allow published_state on scripts to be null
3. On levelbuilder, null out the published_state of any scripts in unit groups.
   • Script.all.select {|s| s.unit_group.present? && s.published_state == s.unit_group.published_state }.each {|s| s.published_state = nil ; s.save! ; s.write_script_dsl ; s.write_script_json }
4. (Don't set script published state on course edit #41617, still in draft) Remove the code that sets the published_state and pilot_experiment on scripts within unit groups

(Steps below may not be done by me)

5. Update the course overview page to handle scripts that have a different published_state than its unit group
6. Add the published_state dropdown back to the script edit page for scrips within courses

ARCHIVE:

1. (This PR) Read published_state and pilot_experiment from the unit_group if a script is in a unit_group. Because these are set to be the same from the course edit page, there should be no user-facing impact.
2. (Allow the script published_state to be null #41616) Allow published_state on scripts to be null
3. (Don't set script published state on course edit #41617, still in draft) Remove the code that sets the published_state and pilot_experiment on scripts within unit groups
4. On levelbuilder, null out the published_state of any scripts in unit groups.
   • Script.all.select {|s| s.unit_group.present? }.each {|s| s.published_state = nil ; s.save! ; s.write_script_dsl ; s.write_script_json }

(Steps below may not be done by me)

5. Update the course overview page to handle scripts that have a different published_state than its unit group
6. Modify get_published_state (the method added in this PR) to look for the script's published_state before it's unit group
7. Add the published_state dropdown back to the script edit page for scrips within courses

Links

Testing story

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR 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

[bethanyaconnor]

I don't know why I needed to do this in so many tests. Anyone have any ideas?

[davidsbailey]

I'm not sure where this is documented, but my general understanding is like this:

1. ActiveRecord objects like script here cache what they know about the state of the DB
2. if you use the object to update the contents of the DB as with script.update!, the cached data in the model is updated (in addition to the DB contents).
3. however, in this case, CourseOffering.add_course_offering calls CourseVersion.add_course_version, which updates an associated object of the script object without actually touching the script object:

   code-dot-org/dashboard/app/models/course_version.rb

   Line 73 in 32172ea

   course_version = CourseVersion.find_or_initialize_by(

   therefore, the script object has stale cached contents, and still thinks it does not have a course_version.
4. if instead CourseVersion.add_course_version looked like this, you might not need to reload after:

  def self.add_course_version(course_offering, content_root)
    if content_root.is_course?
      content_root.course_version = ...

[davidsbailey]

it is also possible you could keep the add_course_version method how it is and add a content_root.reload_course_version call to the end of it, which I just read about here: https://guides.rubyonrails.org/v5.2/association_basics.html#methods-added-by-has-one-association

[bethanyaconnor]

That makes sense as it seems to be happening in other tests as a result of create :unit_group_unit which would also not update the active record cache. Curious why these two methods are causing this in so many cases such as this one.