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

Redesigned entry and login flow #83

Merged
merged 12 commits into from
Jun 5, 2024

Conversation

kyle1morel
Copy link
Collaborator

@kyle1morel kyle1morel commented May 23, 2024

Description

Implements the new login flow. https://apps.nrs.gov.bc.ca/int/jira/browse/PADS-158

  • Hooks PCNS up to the SSO API. If a new user logs into the system who does not have any granted roles, they are assigned the basic role.
  • Adds basic RBAC. This is enough to get the flow working, but RBAC is not implemented through the entire application.
  • New proponent only views
  • New navbar
  • Ability for developers to view the application pretending to be a specific role
  • Various minor updates through application

e0041b9 Permission map updates. Various form text update
bbe9b3a SSO API connection. Role access mappings. Role override ability.
2c2f9d6 Updated navigational flow for all user roles
6d595c1 Add proponent Housing landing page. New navbar functionality
1a75801 Update HomeView

Types of changes

New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING doc
  • I have checked that unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

Copy link

github-actions bot commented May 23, 2024

Coverage Report (Application)

Totals Coverage
Statements: 43% ( 709 / 1649 )
Methods: 33.54% ( 106 / 316 )
Lines: 55.4% ( 477 / 861 )
Branches: 26.69% ( 126 / 472 )

@kyle1morel kyle1morel force-pushed the feature/login-flow branch 5 times, most recently from 09ed184 to 2f31eb2 Compare May 28, 2024 00:48
Copy link

github-actions bot commented May 28, 2024

Coverage Report (Frontend)

Totals Coverage
Statements: 30.27% ( 1081 / 3571 )
Methods: 26.99% ( 197 / 730 )
Lines: 33.71% ( 653 / 1937 )
Branches: 25.55% ( 231 / 904 )

@kyle1morel kyle1morel closed this May 28, 2024
@kyle1morel kyle1morel reopened this May 28, 2024
@kyle1morel kyle1morel force-pushed the feature/login-flow branch 4 times, most recently from 29db1ed to bbe9b3a Compare May 28, 2024 06:40
@slhurley
Copy link
Collaborator

slhurley commented May 28, 2024

Looking amazing! Just a couple of textual changes mainly:

  • 1st button - "what permits are needed" should say "what permits may be needed" and remove the "with a Navigator" part (see Figma)
  • Buttons don't change color and become hyperlinks instead...Figma talked about color changes instead of hyperlinks to activate a button
  • SHAS Form - wording changed from Applicant to Contact (see Figma for PADS-84)
  • SHAS Form - we were going to add the project location description new field under the optional Location area ("Is there anything else you would like to tell us about this project's location?"
  • Enquiry Form - we have to get rid of the PermitConnect Navigator service wording. "Is this enquiry related to an existing PermitConnect Housing Navigator Service application?"....should be "Is this enquiry related to an existing project that you are working on with a Navigator?". The "would you like to apply for...." should be "Would you like to register for Navigator assistance with a project?" And "Enter the confirmation ID of your PCNS application..." would be "Enter the confirmation ID given to you when you registered your project with a Navigator"

@kyle1morel kyle1morel closed this May 29, 2024
@kyle1morel kyle1morel reopened this May 29, 2024
@kyle1morel kyle1morel force-pushed the feature/login-flow branch from c5d8355 to e0041b9 Compare May 29, 2024 20:36
@kyle1morel kyle1morel changed the title WIP login flow Improved login flow May 29, 2024
@kyle1morel kyle1morel changed the title Improved login flow Redesigned entry and login flow May 29, 2024
@kyle1morel kyle1morel marked this pull request as ready for review May 29, 2024 20:53
@slhurley
Copy link
Collaborator

I'm not sure if this comment belongs on this PR or a different one but when doing an Enquiry it loses the name, then saves as a Draft even though it was submitted, and is missing the Submission Type (should auto-set to General Enquiry...perhaps that is why it thinks it is a draft?)

@Subin1Doo
Copy link

Subin1Doo commented May 31, 2024

Things are looking great! I did a full review of all the pages I could see. Some changes / adjustments for UI and wording changes please :)

Homepage

  • Make background color behind text darker, increase opacity by a little bit
    Housing page
  • Add a banner behind housing heading (like in Figma)
  • Add drop shadow to all four buttons
  • Move all four buttons down a little bit so it’s centre-aligned on the page
  • Increase the icon size in all four buttons
  • Hover/ click on a button -> blue background and white font (in Figma)

Updates/ changes to the actual form

  • Notice of collection message only needs to be on basic info tab
  • Under housing tab of form: change “Type in project name…” -> “Type in project..”

Location tab

  • “Has the location of the project been affected…” change to -> “Has the location of this project…”
  • Parcel Id -> LTSA PID Lookup: missing hyperlink
    o https://ltsa.ca/property-owners/about-land-records/property-information-resources/
  • “Is there anything else you would like to tell us about this project’s location” field is optional
  • Confirmation page for both the form and enquiry: success message banner: fix spelling - “successfully.”

- Add labels to all drop-down and text fields, placeholders -> labels

Enquiry:

  • "Would you like to register for Navigator assistance with a project?" -> change to "Would you like to register your project with a Navigator?" (checked with Sharolyn)
  • Tool tip for the above section, change the word "applying" to -> "registering."
  • the blue text that appears when they select "yes" to the above section: "Please proceed to the next page. It will take you to the Single Housing Application." -> change to "Please proceed to the next page to register your project with a Navigator."

Confirmation message when they try to proceed to the actual form

  • Change to “After confirming, your enquiry will be submitted, and you will be directed to register your project with a Navigator.”

Some errors I noticed..

  • When I save both the form and enquiry as a draft, it highlights just the phone number field in red
  • When a user proceeds to the actual form from an enquiry, their contact info gets carried over but not their first and last name.

@Subin1Doo
Copy link

For this comment: Under housing tab of form: change “Type in project name…” -> “Type in project..”
I meant to say change it to "Project name - well known title like Capital Park"
My mistake! Sorry

And, the illustration (png) next to Housing heading is too big right now, can you decrease the size so it's slightly bigger than the "housing" heading.

Thanks!

router.push({ name: RouteNames.HOME });
}
"
>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should pull this out of the template into its own function when three lines or more.

![RouteNames.OIDC_LOGIN, RouteNames.OIDC_CALLBACK, RouteNames.OIDC_LOGOUT].includes(
router.currentRoute.value.name as any
)
"
Copy link
Contributor

Choose a reason for hiding this comment

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

I would pull this logic out of the template when it gets this complex.

@@ -12,7 +12,7 @@ import type { AxiosInstance, AxiosRequestConfig, InternalAxiosRequestConfig } fr
*/
export function appAxios(options: AxiosRequestConfig = {}): AxiosInstance {
const instance = axios.create({
baseURL: new ConfigService().getConfig().apiPath,
baseURL: window.location.origin + `/${new ConfigService().getConfig().apiPath}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: How did ConfigService get the root URL before just realized apiPath before was only /api/v1? And we're not configuring the root URL to be a ENV variable?

@@ -33,7 +33,7 @@ afterEach(() => {
sessionStorage.clear();
});

describe('LoginButton.vue', () => {
describe.skip('LoginButton.vue', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this skip intentional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, for now, struggling to fix these tests with the changes implemented.

@kyle1morel kyle1morel force-pushed the feature/login-flow branch 2 times, most recently from cd2ae86 to 4a7154b Compare June 4, 2024 16:55
@kyle1morel kyle1morel force-pushed the feature/login-flow branch from 1b3371b to 3992725 Compare June 4, 2024 18:38
@Subin1Doo
Copy link

Only got to look at the "My drafts and submissions" page now, sorry for the late comment!

  • can we increase the height of the sorting bar, make it the same height as the sorting bar on the new files tab design
  • Action column doesn't need the sort button/action
  • can we push the action column further to the right while keeping the icons centre-aligned with the word Action
    Thank you!

@kyle1morel kyle1morel force-pushed the feature/login-flow branch from f681736 to 9bc061c Compare June 4, 2024 22:10
@kyle1morel kyle1morel merged commit aa62bc7 into release/housing-intake Jun 5, 2024
16 of 17 checks passed
@kyle1morel kyle1morel deleted the feature/login-flow branch June 5, 2024 19:20
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.

4 participants