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

Create Course: Filter out unversioned families #46084

Merged
merged 12 commits into from
May 7, 2022

Conversation

dmcavoy
Copy link
Contributor

@dmcavoy dmcavoy commented Apr 28, 2022

When creating a course (unit group or standalone unit) you are asked to pick or input a family name. The current dropdown lists all the family names available for that type of course (unit group or standalone unit). However we have families where there are one course in it and its version year is unversioned for those families we do not want to add other versions to the family so this PR removes the family names with unversioned courses in them from the list.

This is follow up work to make sure:

  • You can't input an exist family name
  • Update family names to not end in a year (self-paced-pl-csd-2021 for example)

Links

https://codedotorg.atlassian.net/browse/PLAT-1514

Testing story

  • Tested it out locally
    Current Levelbuilder - New Script:

Screen Shot 2022-05-06 at 1 25 34 PM

Local - New Script:

Screen Shot 2022-05-06 at 1 33 05 PM

Current Levelbuilder - New Course

Screen Shot 2022-05-06 at 1 33 55 PM

Local - New Course

Screen Shot 2022-05-06 at 1 34 19 PM

@dmcavoy dmcavoy changed the title Filter out unversioned families Create Course: Filter out unversioned families May 6, 2022
@dmcavoy dmcavoy requested review from davidsbailey and a team May 6, 2022 17:26
@dmcavoy dmcavoy removed request for a team and davidsbailey May 6, 2022 17:29
@dmcavoy dmcavoy requested review from davidsbailey and a team May 6, 2022 17:34
@dmcavoy
Copy link
Contributor Author

dmcavoy commented May 6, 2022

Alright. Ready for review now.

Copy link
Member

@davidsbailey davidsbailey left a comment

Choose a reason for hiding this comment

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

Looks great Dani! In addition to keeping us out of trouble, I love how much shorter these lists are now! much easier to find what you're actually looking for 🔎

@course_families_course_types << [cf, {instruction_type: ug.instruction_type, instructor_audience: ug.instructor_audience, participant_audience: ug.participant_audience}]
end

@course_families_course_types.to_h
Copy link
Member

Choose a reason for hiding this comment

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

can you remind me, does the return value for this method get used somewhere? here and below.

the place I would start looking is https://guides.rubyonrails.org/action_controller_overview.html, but I didn't see anything definitive. if you can't find a way that it's used, and you feel good about your test coverage, I would suggest removing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't actually trying to return @course_families_course_types I just needed for follow up after setting it up to convert it to a hash. I'm going to leave this and merge it but if you have thoughts when you get back I'm happy to follow up

Copy link
Member

Choose a reason for hiding this comment

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

This returns a hash, but does not convert the originals object into a hash, so this statement ultimately looks like it has no effect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I see what I missed. Ok thanks I will look into this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixing it here: #46239

@dmcavoy dmcavoy merged commit 1b154a5 into staging May 7, 2022
@dmcavoy dmcavoy deleted the filter-out-unversioned-families branch May 7, 2022 11:33
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.

None yet

2 participants