bcgov / cas-cif

CleanBC Industry Fund project
Apache License 2.0
6 stars 2 forks source link

`Login Page` link in the bottom navigation bar must address correct environment #925

Closed Sepehr-Sobhani closed 2 years ago

Sepehr-Sobhani commented 2 years ago

Describe the Bug:

currently, the Login Page link in the bottom navigation bar only takes the user to prod environment. we need to change it so if the user is in dev environment, takes the user to dev login page and so on...

Probability (how likely the bug is to happen, scored from 1-5): 5 (For example, probability of 5 is something like "it happens to all users every time they log in." Probability of 1 "only happens to certain users when a really specific and unlikely path is followed.")

Effect (how bad the bug is when it does happen, scored from 1-5): 5 (For example, effect of 5 is "the entire app crashes and makes it unusable for all users" or "the bug causes the wrong data to be saved, with critical information (e.g. payment) being affected." Effect of 1 is "It makes some styling look a little bit weird.")

Steps to reproduce the behaviour:

open the cif app in any environment and click the Login Page link at the bottom navigation.

LindsayMacfarlane commented 2 years ago

Thank you for your quick action on this bug @Sepehr-Sobhani!

One observation - when I select the login page link in the navigation bar, it now opens up the dev environment (as expected), but it takes the user to the home page and not the login page. I think it is supposed to open to the login page?

Sepehr-Sobhani commented 2 years ago

Thank you for your quick action on this bug @Sepehr-Sobhani!

One observation - when I select the login page link in the navigation bar, it now opens up the dev environment (as expected), but it takes the user to the home page and not the login page. I think it is supposed to open to the login page?

Thanks for mentioning that @LindsayMacfarlane . In this case, I'm assuming we need to end the users' session by clicking that link. Am I right @pbastia ?!

pbastia commented 2 years ago

I'm wondering if a mistake snuck in on the wireframe - on other projects like CIIP or GGIRCS, that link is called "Home", this is probably what we want here (then the behaviour is indeed correct).

LindsayMacfarlane commented 2 years ago

But then that is the same functionality as the 'Home' in the top navigation bar. I feel the purpose of the Login on the bottom navigation was to take the user back to the login page. @suhafa - can you confirm?

suhafa commented 2 years ago

thanks for your questions @LindsayMacfarlane @pbastia. The intention here was that the 'Login Page' takes you to this screen (- Screen Shot 2022-08-08 at 12.58.32 PM.png We may utilize this screen in the future for information about CIF

pbastia commented 2 years ago

Wouldn't this mean that we're logging the user out, without the action being explicit anywhere?

suhafa commented 2 years ago

We won't log out the user - clicking on 'login page' will reflect what happens when you click on 'Home' in the test environment currently once logged in. I've replaced the screenshot above with a screenshot of that to illustrate it better. Essentially 'Login Page' Is a stand in for 'Landing Page' which is what you see when you access the website before logging in, and will also be able to view once your are logged in. I'd be happy to jump on a quick call if you'd prefer to lock this down better if needed

suhafa commented 2 years ago

After a sync up with @pbastia we concluded that we don't have a need for this link for now and decided to remove it. A ticket to remove the link will be made and the wireframe in Figma will be updated accordingly

LindsayMacfarlane commented 2 years ago

Is removing the login link from the navigation bar in a separate ticket or being done as part of this one?

Sepehr-Sobhani commented 2 years ago

I can push a quick PR without a card for this one if need be.

pbastia commented 2 years ago

This card already passed review and PO approval, I'd say create a new small one (it's probably a lot less high priority than what we have in the sprint backlog at the moment)

Sepehr-Sobhani commented 2 years ago

Made a tech debt card for that: https://github.com/bcgov/cas-cif/issues/957