Store unversioned scripts in other directory

[smusoke]

What: The directory that scripts with a version_year of unversioned are saved in - won't be the same as versioned scripts.

Why: Unversioned scripts were being skipped with the existing logic

Links

• jira ticket: FND-1666

Testing story

Ran sync-in.rb locally and no scripts were skipped

Before:

After:

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

[Hamms]

This change looks good! Should definitely preserve consistency, but I have one nit about the implementation. I'd rather see this logic laid out in a way that makes the most sense given the new logic, rather than in a way that most easily fits into the existing layout.

I'm thinking something like

# in script model
def unversioned?
  version_year.blank? || version_year == 'unversioned'
end

      script_i18n_directory =
        if ScriptConstants.unit_in_category?(:hoc, script.name)
          File.join(level_content_directory, "Hour of Code")
        elsif script.unversioned?
          File.join(level_content_directory, "other")
        else
          File.join(level_content_directory, script.version_year)
        end

[smusoke]

This change looks good! Should definitely preserve consistency, but I have one nit about the implementation. I'd rather see this logic laid out in a way that makes the most sense given the new logic, rather than in a way that most easily fits into the existing layout.

I'm thinking something like

# in script model
def unversioned?
  version_year.blank? || version_year == 'unversioned'
end

      script_i18n_directory =
        if ScriptConstants.unit_in_category?(:hoc, script.name)
          File.join(level_content_directory, "Hour of Code")
        elsif script.unversioned?
          File.join(level_content_directory, "other")
        else
          File.join(level_content_directory, script.version_year)
        end

@Hamms I agree that's a better solution long term. Made the update in the latest commit.

[Hamms]

One final nit, then LGTM!

[Hamms]

I poked around a bit more, and it looks like we actually have a constant for this!

Suggested change

	version_year.blank? || version_year == 'unversioned'
	version_year.blank? || version_year == CourseVersion::UNVERSIONED

[smusoke]

Ah thanks for finding this!