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

add query param for chosen school on programs route #7935

Merged

Conversation

michaelchadwick
Copy link
Contributor

Copy link
Member

@jrjohnson jrjohnson left a comment

Choose a reason for hiding this comment

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

Needs an acceptance test for the original bug where you do exactly the steps in the report. I'd add that into packages/frontend/tests/acceptance/programs-test.js.

@@ -41,8 +40,8 @@ export default class ProgramRootComponent extends Component {
}

get bestSelectedSchool() {
if (this.selectedSchoolId) {
return findById(this.args.schools.slice(), this.selectedSchoolId);
Copy link
Member

@jrjohnson jrjohnson Jul 5, 2024

Choose a reason for hiding this comment

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

This slice() used to be needed as ember-data returned a thing that wasn't quite an array, however those days are gone and this can go away too and should do the same thing. Unless I'm wrong. Which isn't impossible.

@michaelchadwick
Copy link
Contributor Author

Needs an acceptance test for the original bug where you do exactly the steps in the report. I'd add that into packages/frontend/tests/acceptance/programs-test.js.

This seems like a great idea. Just as an aside, I don't see any kind of test for this kind of thing in the main Courses test file, but maybe it's located somewhere else.

@michaelchadwick michaelchadwick changed the title add query param for chosen school add query param for chosen school on programs route Jul 8, 2024
@michaelchadwick
Copy link
Contributor Author

@jrjohnson Changes requested exist in the same file outside of the PR, so will follow up with a PR after this that fixes that for all of them.

@dartajax dartajax merged commit bac6e27 into ilios:master Jul 8, 2024
27 checks passed
@michaelchadwick michaelchadwick deleted the frontend-3457-fix-back-to-programs-link branch July 8, 2024 20:36
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.

Back To Programs Link Doesn't Maintain School Selection
3 participants