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

Dev → Main March 2024 #209

Merged
merged 18 commits into from Mar 13, 2024
Merged

Dev → Main March 2024 #209

merged 18 commits into from Mar 13, 2024

Conversation

josh-chamberlain
Copy link
Contributor

Testing user login, maybe other things.

Fixes

Contains PRs

Docs

  • still need to make docs for this; API docs should be set

Marty Bode and others added 14 commits February 9, 2024 08:20
…or/search-results-and-quick-search

refactor: search results and quick search
* wip

* user login changes

* missing file

* missing file

* missing file

* add user put test

* Dev -> Main (#202)

* Add last cached value to data source query

* test: update snapshots

* fix(assets): favicon not displaying

* feat(pages): update DataSourceStaticView

Fix url breaking, add support for last archived, add button to go to Internet Archive

* test(pages): update tests and snapshots

Update test for DataSourceStaticView, update various snapshots

* chore(lint): reformat files with lint errors using black

* chore(lint): attempt re-formatting again with actual VS 'black' extension

* chore: update .gitignore

* refactor(util): update formatDate to return undefined for invalid date passed

* test(util): update test for formatDate

* get_data_source_by_id fix

* chore(deps): add eslint-config from design-system

* test(scripts): update test scripts

* docs(readme): document lint/test script updates

* refactor: update miscellaneous files with linting errors

* test(snapshots): update outdated snapshots

* ci(client): add client scripts to pull workflow

* chore(config): remove stray eslint config from package.json

* ci: move working-directory to top level defaults

* ci: add cache-dependency-path to setup-node action

* ci: add cache-dependency-path to all steps

* chore(linting): remove extraneous rules in linting config
update files per update

* chore(scripts): update ci script to use proper flag

* Revert "test(snapshots): update outdated snapshots"

This reverts commit 386d09b.

* test: update snapshots again

* ci: use exact node version used locally

* Revert "ci: use exact node version used locally"

This reverts commit e1c7b73.

* chore(deps): re-install deps with node v20

* ci: use node v20

* test: update snapshots

* ci: add time zone setter to test script

* feature: add tertiary button from design-system

* test(pages): update DataSourceStaticView test

* chore(deps): bump design-system -> 2.2.0

* fix: miscellaneous styling issues

* test: update snapshots

* chore(deps): bump design-system -> 2.3.0

* chore(cleanup): remove logs and miscellaneous updates

* test: update snapshots

* remove agencies join from archives endpoint, change url_status when updating broken url

* update test columns

* standardized error codes

---------

Co-authored-by: kalenluciano <103911842+kalenluciano@users.noreply.github.com>
Co-authored-by: Joshua Graber <joshua.d.graber@gmail.com>
Co-authored-by: Joshua Graber <68428039+joshuagraber@users.noreply.github.com>
Co-authored-by: Marty Bode <martybode@gmail.com>

* chore(deps): bump design-system 2.3.0 -> 2.4.0

* chore(deps): bump design-system -> 2.4.1

* feat(pages): Add login page

* feat(pages): add shell of password page

* feat(router): add new pages to router

* reset password, role check for edit permissions

* missing file

* fixed tests

* fixed tests

* don't insert search log on test

* move login to own endpoint

* session token

* refresh session tokens

* style(pages): miscellaneous code cleanup

* chore(deps): bump vue, add pinia

* chore(deps): add jwt-decode

* chore(deps): add lodash

* refresh test and fix

* chore(deps): add persist state plugin for pinia

* feat(state): add pinia store for auth
Add pinia and store,
Update router to protect auth routes,
Add util to parse JWT

* feat(components): add AuthWrapper component
Listen for user interactions and automatically refresh token

* refactor(pages): update login page
Use composition API,
Use new auth store

* refactor(router): push to login rather than update route

* docs(README): update client docs

* unit tests

* chore(deps): pinia testing

* test(pages): update tests and snapshots

* test(util): add test for parseJwt

* test(components): add test for AuthWrapper

* fix(router): public route logic conflicting
Check private routes instead

* chore(router): cleanup: remove unnecessary check

* fix(store): update logout patch

* fix(store): one more update to logout

* feat(stores): create user store
move signup func
add password change / reset logic
update LogIn

* feat(pages): build change password route

* feat(pages): add password reset route

* refactor(store): log user back in on pw change

* fix(stores): status code logic

* PR feedback changes

* refactor: miscellaneous updates
Error handling / clearing
Auth header for change pw route

* PR feedback changes

* chore(local): standardize port for dev server

* refactor: update password reset
Update base url vars,
Update user async funcs

* docs(README): update client notes

* refactor(pages): update log in per feedback

* test fix

* test(components): update AuthWrapper test

* test(pages): add LogIn tests

* chore(cleanup): remove stray log

* test(pages): update login snapshots

* fix(ci): indentation in test.yml

* test fix

* cleanup

* Update pull.yaml

* Update login_queries.py

* Update pull.yaml

* Update pull.yaml

* test(pages): update login test -> coverage 100%

* test(pages): add ChangePassword test

* test(pages): misc updates to login and change password tests

* test(pages): add tests for ResetPassword

* update login queries

* test(components): search results card -> 100% coverage

* test(pages): miscellaneous updates to static view

* ci(pull): update env setting in API test

* docs(README): add local client base url

---------

Co-authored-by: Marty Bode <martybode@gmail.com>
Co-authored-by: kalenluciano <103911842+kalenluciano@users.noreply.github.com>
Co-authored-by: Joshua Graber <joshua.d.graber@gmail.com>
Co-authored-by: Joshua Graber <68428039+joshuagraber@users.noreply.github.com>
@josh-chamberlain
Copy link
Contributor Author

Testing login at https://data-sources.pdap.dev/login and the forms just refresh. Could be an envar issue, based on the undefined/user in the route? @mbodeantor

Screen Shot 2024-03-12 at 1 45 55 PM

@josh-chamberlain
Copy link
Contributor Author

@joshuagraber based on the above, I'm thinking we should have a catch-all error value for unpredictable issues, like:

Something went wrong on our end. If you keep seeing this message, please email contact@pdap.io for assistance.

@mbodeantor
Copy link
Contributor

@josh-chamberlain just updated the env vars

@joshuagraber
Copy link
Contributor

based on the above, I'm thinking we should have a catch-all error value for unpredictable issues, like:

Sounds good @josh-chamberlain I'll add an issue to add an error boundary component to render in case of any unexplained/uncaught API errors like this.

@josh-chamberlain
Copy link
Contributor Author

Thanks @joshuagraber and @mbodeantor. I noticed just a couple more things:

  • When the passwords don't match, we get a "Please provide your password" error which should probably read "Passwords must match". (image below)

  • My password reset email took maybe 10 minutes, but by the time it showed up, the link took me to an expired token page. (image below). Can we extend that reset token to maybe 15–20 minutes, and add a line to the email about how long it'll take to expire?

  • On the token expired page, instead of columns, can we just have the line of text with a Request another password reset button below it?

Screen Shot 2024-03-12 at 4 22 18 PM Screen Shot 2024-03-12 at 4 36 23 PM

@joshuagraber
Copy link
Contributor

joshuagraber commented Mar 13, 2024

Hey @josh-chamberlain, no probs.

A couple thoughts/questions:

When the passwords don't match, we get a "Please provide your password" error which should probably read "Passwords must match". (image below)

The error in your screenshot is the password field validation (does PW meet all the required conditions), not the password mismatch error (see screenshot below). Input validation happens before we check that the passwords match, so you'll never get to the PW match validation until both inputs contain a valid password.

Screenshot 2024-03-13 at 6 01 09 AM

(Another thing: I just noticed the form inputs clear on the password mismatch error, but that's a design-system problem. Will create an issue to preserve form input values after onSubmit validation errors in parent components.)

My password reset email took maybe 10 minutes, but by the time it showed up, the link took me to an expired token page. (image below). Can we extend that reset token to maybe 15–20 minutes, and add a line to the email about how long it'll take to expire?

Agreed. It would also be nicer UX if we have an endpoint to validate this token on page load, so the user doesn't have to fill out the form and submit to know that it's expired. @mbodeantor Do you think it makes sense to add an issue to tackle this later, as an enhancement to the reset pw experience?

On the token expired page, instead of columns, can we just have the line of text with a Request another password reset button below it?

Done.

I also started the error boundary component and catchall "not found" route from #210 yesterday, so added that stuff here as well.
PR is here, if you want to add any related API changes @mbodeantor.

@josh-chamberlain
Copy link
Contributor Author

↑ this is good for me, @mbodeantor if you want to get the other one in dev, and I added another issue so we could merge this real soon: #214

@mbodeantor mbodeantor merged commit fccf933 into main Mar 13, 2024
8 checks passed
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.

None yet

3 participants