Fix outdated resource and vocab links

[bethanyaconnor]

I wrote a script to find and replace instances of resource and vocab text that didn't match the course version of the lesson. I don't believe I caught every single piece of markdown that needed to be updated. My plan is to see if I can get a list of the rest and either update myself or pass to the curriculum team to fix.

I spot checked a couple of the times the resource key changed in the markdown and they are existing resources with a slightly different key. The dedup logic is based on name and url of the resource so I think these keys just got changed along the way.

Script (note that there is some logic to prevent writing every single script_json file because that was a problem):

require_relative '../../dashboard/config/environment'

Script.all.each do |script|
  next unless script.name == 'csp1-2022'
  next unless script.get_course_version
  next if script.use_legacy_lesson_plans
  cv = script.get_course_version
  script.lessons.each do |lesson|
    next unless lesson.has_lesson_plan
    update_resource_link = proc do |resource|
      new_resource =  resource.course_version == cv ? resource : resource.copy_to_course_version(cv)
      "[r #{new_resource ? Services::GloballyUniqueIdentifiers.build_resource_key(new_resource) : Services::GloballyUniqueIdentifiers.build_resource_key(resource)}]"
    end

    update_vocab_definition = proc do |vocab|
      new_vocab =  vocab.course_version == cv ? vocab : vocab.copy_to_course_version(cv)
      "[v #{new_vocab ? Services::GloballyUniqueIdentifiers.build_vocab_key(new_vocab) : Services::GloballyUniqueIdentifiers.build_vocab_key(vocab)}]"
    end

    changed = false
    %w(overview student_overview preparation assessment_opportunities purpose).each do |field|
      next unless lesson.try(field)
      previous = lesson.try(field)
      Services::MarkdownPreprocessor.sub_resource_links!(lesson.try(field), update_resource_link)
      Services::MarkdownPreprocessor.sub_vocab_definitions!(lesson.try(field), update_vocab_definition)
      changed ||= (previous != lesson.try(field))
    end
    changed ||= lesson.changed?
    lesson.save! if lesson.changed?
    Script.merge_and_write_i18n(lesson.i18n_hash, script.name) if changed

    lesson.lesson_activities.each do |lesson_activity|
      lesson_activity.activity_sections.each do |activity_section|
        if activity_section.description
          previous = activity_section.description.dup
          puts previous
          Services::MarkdownPreprocessor.sub_resource_links!(activity_section.description, update_resource_link)
          Services::MarkdownPreprocessor.sub_vocab_definitions!(activity_section.description, update_vocab_definition)
          puts activity_section.description
          changed ||= (previous != activity_section.description)
        end
        changed = changed || activity_section.changed?
        unless activity_section.tips.blank?
          previous_tips = activity_section.tips.dup.compact
          previous_serialization = previous_tips.to_json
          activity_section.tips = previous_tips.map do |tip|
            if tip['markdown']
              Services::MarkdownPreprocessor.sub_resource_links!(tip['markdown'], update_resource_link)
              Services::MarkdownPreprocessor.sub_vocab_definitions!(tip['markdown'], update_vocab_definition)
            end
            tip
          end
          changed = changed || (previous_serialization != activity_section.tips.to_json)
          activity_section.save! if activity_section.changed?
        end
      end
    end
    script.write_script_json if changed
  end
end

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]

Note: because of the ongoing Drone problems, I might merge before Drone succeeds so that this PR doesn't become stale. It already has a high change of merge conflicts and if it gets stale, that chance will increase.

[davidsbailey]

this looks great, thank you for doing this Bethany! my main question is whether there's any additional follow-up work we should be tracking to prevent these kinds of errors from popping up again in the future.

[davidsbailey]

why is the resource key ('ab-cutouts') changing here -- are we regenerating the key from the resource name when we copy?

[bethanyaconnor]

It looks like we are. I double checked this specific change and the 2022 and 2021 resources do look the same.

We could definitely change the copy_to_course_version logic to try to preserve the key if we wanted.

[davidsbailey]

thank you for the pointer! the current behavior sounds good to me.

[davidsbailey]

is there any follow-up needed to prevent this kind of problem from happening again in the future? it seems like an issue with lesson copy, but not sure if it's since been addressed

[bethanyaconnor]

I addressed issues in #45979 and #46114, but I think there might be one more to come (I forgot to add purpose to the list of fields to update). Each time, I'm adding more tests but with issues like forgetting to update a field, I'm not sure the best way to ensure they're not forgotten

[davidsbailey]

I see. Here is an idea:

1. extract this into a constant array MARKDOWN_FIELDS:

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

   Line 829 in 5e0e00b

   %w(overview student_overview preparation assessment_opportunities).each do |field|

2. make this method raise if you try to render something that is not in the new MARKDOWN_FIELDS array:

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

   Line 175 in 5e0e00b

   def render_property(property_name)

happy to own follow-up if that feels like too much.