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.

Sign up for GitHub

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

Jump to bottom

Fix app login redirect bug #1031

Merged
merged 3 commits into from
May 15, 2023
Merged

Fix app login redirect bug #1031

merged 3 commits into from
May 15, 2023

Conversation

curtisupshall
Copy link
Contributor

@curtisupshall curtisupshall commented May 11, 2023 •
edited by al-rosenthal
Loading

Overview

Fixes a bug that prevents users from being correctly redirected to the login page

Reproducing the Bug

  1. Go to SIMS dev, or run SIMS locally on the dev branch
  2. Sign in to SIMS.
  3. Once signed in, change your path to /login. For example, localhost:7100/login, or https://sims-dev-url.gov.bc.ca/login
  4. You should:
  • See a blank white screen.

Testing the hotfix

  1. Checkout the hotfix branch, or visit the PR app route at: https://biohubbc-app-1031-af2668-dev.apps.silver.devops.gov.bc.ca/
  2. Sign in to SIMS.
  3. Once signed in, change your path to `/login'.
  4. You should:
  • Be redirected to the SIMS home page.
  1. Next, change your path to /login?redirect=%2Fadmin%2Fprojects
  2. You should:
  • Be redirected to the SIMS projects list page.

@github-actions
Copy link

Openshift URLs for the PR Deployment:

@codecov
Copy link

codecov bot commented May 11, 2023 •
edited
Loading

Codecov Report

Merging #1031 (3185636) into dev (fde5a51) will increase coverage by 0.02%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##              dev    #1031      +/-   ##
==========================================
+ Coverage   68.19%   68.22%   +0.02%     
==========================================
  Files         387      386       -1     
  Lines       12037    12032       -5     
  Branches     2061     2061              
==========================================
  Hits         8209     8209              
+ Misses       3322     3317       -5     
  Partials      506      506
Impacted Files Coverage Δ
app/src/components/security/RouteGuards.tsx 0.00% <0.00%> (ø)
app/src/hooks/useRedirect.tsx 57.14% <ø> (ø)
app/src/pages/authentication/LoginPage.tsx 100.00% <100.00%> (+100.00%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

KjartanE
KjartanE previously approved these changes May 11, 2023
View reviewed changes
Copy link
Contributor

@KjartanE KjartanE left a comment

Choose a reason for hiding this comment

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

🐈

@curtisupshall curtisupshall added the Do Not Merge PR should not be merged label May 11, 2023
@curtisupshall curtisupshall dismissed stale reviews from anissa-agahchen and KjartanE via 0272301 May 11, 2023 17:44
@curtisupshall curtisupshall removed the Do Not Merge PR should not be merged label May 11, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@curtisupshall curtisupshall requested a review from NickPhura May 15, 2023 16:02
al-rosenthal
al-rosenthal reviewed May 15, 2023
View reviewed changes
Copy link
Contributor

@al-rosenthal al-rosenthal left a comment

Choose a reason for hiding this comment

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

Code looks good, not sure how to reproduce the issue in the first place though

@curtisupshall
Copy link
Contributor Author

curtisupshall commented May 15, 2023 •
edited
Loading

Code looks good, not sure how to reproduce the issue in the first place though

Thanks for the review @al-rosenthal. I've updated the PR description to include testing procedures. :^)

al-rosenthal
al-rosenthal approved these changes May 15, 2023
View reviewed changes
Copy link
Contributor

@al-rosenthal al-rosenthal left a comment

Choose a reason for hiding this comment

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

🐅

KjartanE
KjartanE approved these changes May 15, 2023
View reviewed changes
Copy link
Contributor

@KjartanE KjartanE left a comment

Choose a reason for hiding this comment

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

🐈

@curtisupshall curtisupshall merged commit 68469e8 into dev May 15, 2023
@curtisupshall curtisupshall deleted the redirect-hotfix branch May 15, 2023 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KjartanE KjartanE KjartanE approved these changes

@al-rosenthal al-rosenthal al-rosenthal approved these changes

@AlfredRosenthal AlfredRosenthal Awaiting requested review from AlfredRosenthal

@anissa-agahchen anissa-agahchen Awaiting requested review from anissa-agahchen

@NickPhura NickPhura Awaiting requested review from NickPhura

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@curtisupshall @al-rosenthal @anissa-agahchen @KjartanE