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

Fix searching for levels with name blockly #42916

Merged
merged 2 commits into from
Oct 11, 2021
Merged

Conversation

dmcavoy
Copy link
Contributor

@dmcavoy dmcavoy commented Oct 8, 2021

Dan and Mike pointed out that every old style blockly level was showing with blockly as its name in the AddLevelsDialog. The name we actually want to use for these levels is in the level_num. This updates the search and levels list to take level_num into account for these levels.

Screen Shot 2021-10-08 at 5 24 17 PM

Links

Slack: https://codedotorg.slack.com/archives/C02EEGWLHR8/p1633726284200100

Testing story

  • Tested locally
  • Existing Tests
  • New unit tests

@dmcavoy dmcavoy requested a review from a team October 10, 2021 14:43
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.

this works fine for word search, but the results are pretty confusing if I am looking for a level like blockly:Artist4:3_1:

Screen Shot 2021-10-10 at 1 11 29 PM

Screen Shot 2021-10-10 at 1 12 28 PM

part of the problem is that the "type" of these levels is Blockly, not Artist. another part of the problem is that _ appears to be a wildcard in mysql LIKE syntax.

What I would suggest is, when the search string starts with blockly:, just look for one level where the level key exactly matches the search string, using Level.find_by_key. I can't think of a situation where a curriculum writer would want to add one of these levels, but wouldn't already have the entire level key available.

@dmcavoy
Copy link
Contributor Author

dmcavoy commented Oct 11, 2021

@davidsbailey Thanks for the feedback. That all makes sense and I can definitely update. Do you mind if I get this change in since curriculum needed this fixed asap and then I can update it to account for your feedback?

@davidsbailey
Copy link
Member

@davidsbailey Thanks for the feedback. That all makes sense and I can definitely update. Do you mind if I get this change in since curriculum needed this fixed asap and then I can update it to account for your feedback?

@dmcavoy yep, I think that makes sense given the situation.

@dmcavoy dmcavoy merged commit 8f5503a into staging Oct 11, 2021
@dmcavoy dmcavoy deleted the fix-blockly-in-add branch October 11, 2021 13:21
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