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

check for primary script before redirect #18572

Merged
merged 1 commit into from Oct 23, 2017
Merged

Conversation

Erin007
Copy link
Contributor

@Erin007 Erin007 commented Oct 23, 2017

I introduced a bug in #18493. This change confirms that the user does have a primary script before trying to redirect them to their primary script's course overview page.

@Erin007
Copy link
Contributor Author

Erin007 commented Oct 23, 2017

cc: @tanyaparker

Copy link
Contributor

@Bjvanminnen Bjvanminnen left a comment

Choose a reason for hiding this comment

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

What's the user impact here? Does this justify a hotfix?

@Erin007
Copy link
Contributor Author

Erin007 commented Oct 23, 2017

User bad is 5xx error if you have a newly assigned course or script, but no progress in any courses/scripts. It wants to redirect you to the primary script overview page, but can't because it doesn't know what the id of the primary script is. It's been tricky to repro exactly because it only seems to affect some courses/scripts.

@Bjvanminnen
Copy link
Contributor

5xx error describes what happens on our end - it would be good to also know what the user sees when they run into this error. I suspect we would have received zendesk tickets if this was too painful an experience, so I will operate under the assumption that shipping this as part of the regular DTP is okay until/unless I hear otherwise.

Copy link
Contributor

@tanyaparker tanyaparker left a comment

Choose a reason for hiding this comment

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

Agree with Brent. Checked with Josh Schulte to see if he's heard anything. He hasn't, so let's go with putting this in with today's DTP.

@Erin007 Erin007 merged commit e1da902 into staging Oct 23, 2017
@Erin007 Erin007 deleted the redirect-to-primary-script branch October 23, 2017 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants