fix: auth error handling #144

Sparkenstein · 2024-10-24T16:33:28Z

Error handling on signup page. haven't checked login yet.

* udated clone url to make it clearer that it is a fork * bolded section titles

* chore: release v0.4.1-hotfix.3 * fix(data-migration): early return if no categories have parents (goniszewski#128) Signed-off-by: Robert Goniszewski <robertgoniszewski@outlook.com> * Closes goniszewski#130 (goniszewski#131) Signed-off-by: Robert Goniszewski <robertgoniszewski@outlook.com> * fix(database): use dynamic path for SQLite database file Signed-off-by: Robert Goniszewski <robertgoniszewski@outlook.com> * docs(readme): use single README file for latest/preview version Signed-off-by: Robert Goniszewski <robertgoniszewski@outlook.com> * feat(ci): add manual deployment workflow and adjust tag conditions Signed-off-by: Robert Goniszewski <robertgoniszewski@outlook.com> --------- Signed-off-by: Robert Goniszewski <robertgoniszewski@outlook.com>

Sparkenstein · 2024-10-24T16:34:26Z

Solves #143

Sparkenstein · 2024-10-25T16:42:37Z

CONTRIBUTING.md

@@ -2,26 +2,35 @@

 Thank you for considering contributing to this project! All contributors, big or small, are welcomed. To make the contribution process as smooth as possible, please follow the guidelines below.

+## Prerequisites


These steps are missing to get started for a new developer. if you want I can create separate PR for this

Nah, let's ship it with this PR. Thanks!

Sparkenstein · 2024-10-25T16:43:59Z

src/routes/login/+page.server.ts

@@ -16,17 +17,10 @@ export const actions: Actions = {
 		const existingUser = await getUserWithoutSerialization(login);

 		if (!existingUser) {
-			const randomMs = Math.floor(Math.random() * 1000);


Not sure what was the point of this code.

Why random wait?

The function is already async it will always return a promise, returning new Promise(....) just adds a bit more work for compiler.

It was here to simulate request time, as immediate response means that the user doesn't exist.

When I think about it know, maybe it was too much precaution on my side :)

goniszewski

These changes are well-considered, just a bit of refinement or further discussion and we’ll be all set!

goniszewski · 2024-10-25T17:05:10Z

CONTRIBUTING.md

@@ -2,26 +2,35 @@

 Thank you for considering contributing to this project! All contributors, big or small, are welcomed. To make the contribution process as smooth as possible, please follow the guidelines below.

+## Prerequisites


Nah, let's ship it with this PR. Thanks!

CONTRIBUTING.md

goniszewski · 2024-10-25T17:09:29Z

src/routes/login/+page.server.ts

@@ -16,17 +17,10 @@ export const actions: Actions = {
 		const existingUser = await getUserWithoutSerialization(login);

 		if (!existingUser) {
-			const randomMs = Math.floor(Math.random() * 1000);


It was here to simulate request time, as immediate response means that the user doesn't exist.

When I think about it know, maybe it was too much precaution on my side :)

goniszewski · 2024-10-25T17:10:34Z

src/routes/signup/+page.server.ts

@@ -41,9 +43,11 @@ export const actions: Actions = {
 		}>({
 			name: Joi.string().min(3).max(31).optional(),
 			username: Joi.string().min(3).max(31),
-			email: Joi.string().email().optional(),
+			email: Joi.string().email().required(),


I'm not yet sure if email is necessary. Not at the time, at least.

It's mandatory on UI tho. So we either make it optional on UI or mandatory here. whichever you prefer. I think optional makes sense because we already have username. But in future if we introduce notification feature then we are gonna need email.

Ok, so maybe let's make it optional for now.

Sparkenstein · 2024-10-27T14:18:11Z

.github/workflows/ci.yml

@@ -17,7 +17,7 @@ jobs:
          images: goniszewski/grimoire
          tags: |
            type=raw,value=latest,enable=${{ !github.event.release.prerelease }}
-            type=raw,value=preview,enable=${{ !github.event.release.prerelease }}
+            type=raw,value=preview,enable=${{ github.event.release.prerelease }}


These are not my changes not sure why they are here

Oh, I did change the target branch to develop, as it needs to be merged there first. Sorry for the confusion!

Co-authored-by: Robert Goniszewski <43510122+goniszewski@users.noreply.github.com>

Sparkenstein · 2024-10-27T14:20:47Z

src/routes/signup/+page.server.ts

@@ -41,9 +43,11 @@ export const actions: Actions = {
 		}>({
 			name: Joi.string().min(3).max(31).optional(),
 			username: Joi.string().min(3).max(31),
-			email: Joi.string().email().optional(),
+			email: Joi.string().email().required(),


It's mandatory on UI tho. So we either make it optional on UI or mandatory here. whichever you prefer. I think optional makes sense because we already have username. But in future if we introduce notification feature then we are gonna need email.

Sparkenstein · 2024-10-27T14:21:18Z

.github/workflows/manual-deploy.yml

@@ -0,0 +1,49 @@
+name: manual-deploy


Not my change as well

sonarcloud · 2024-10-31T13:23:32Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Sparkenstein · 2024-10-31T13:23:33Z

Sorry for the delay, made email optional and resolved merge conflicts. anything else you want to address?

goniszewski · 2024-10-31T19:23:43Z

Sorry for the delay, made email optional and resolved merge conflicts. anything else you want to address?

No problem. Glad to see you had time to address those things.

Thank you for your contribution!

* chore: release v0.4.1-hotfix.3 * fix(data-migration): early return if no categories have parents (#128) Signed-off-by: Robert Goniszewski <robertgoniszewski@outlook.com> * Closes #130 (#131) Signed-off-by: Robert Goniszewski <robertgoniszewski@outlook.com> * fix(database): use dynamic path for SQLite database file Signed-off-by: Robert Goniszewski <robertgoniszewski@outlook.com> * docs(readme): use single README file for latest/preview version Signed-off-by: Robert Goniszewski <robertgoniszewski@outlook.com> * feat(ci): add manual deployment workflow and adjust tag conditions Signed-off-by: Robert Goniszewski <robertgoniszewski@outlook.com> * refactor(workflow): simplify manual-deploy GitHub Action Signed-off-by: Robert Goniszewski <robertgoniszewski@outlook.com> * fix(metadata): handle multiple image URLs in mainImageUrl field Signed-off-by: Robert Goniszewski <robertgoniszewski@outlook.com> * fix: auth error handling (#144) * refactor(api): migrate Swagger UI to external documentation and enhance health endpoint Signed-off-by: Robert Goniszewski <robertgoniszewski@outlook.com> * chore: release v0.4.3 --------- Signed-off-by: Robert Goniszewski <robertgoniszewski@outlook.com> Co-authored-by: Prabhanjan <zetabytes.pp@gmail.com>

Grazerquart and others added 4 commits September 27, 2024 22:17

docs: Minor changes to Contributing.md (goniszewski#121)

4515292

* udated clone url to make it clearer that it is a fork * bolded section titles

chore: release v0.4.2

cc29548

fix: auth error handling

38f85ab

Sparkenstein added 3 commits October 25, 2024 16:37

feat: login error handling

0838443

fix: duplicate user

61a8322

doc: add missing steps for dev-env

2359f05

Sparkenstein commented Oct 25, 2024

View reviewed changes

goniszewski requested changes Oct 25, 2024

View reviewed changes

goniszewski changed the base branch from main to develop October 25, 2024 17:16

goniszewski assigned Sparkenstein Oct 25, 2024

goniszewski added bug Something isn't working documentation Improvements or additions to documentation labels Oct 25, 2024

Sparkenstein commented Oct 27, 2024

View reviewed changes

chore: doc fix

4fc603f

Co-authored-by: Robert Goniszewski <43510122+goniszewski@users.noreply.github.com>

Sparkenstein commented Oct 27, 2024

View reviewed changes

Sparkenstein added 3 commits October 31, 2024 13:18

fix: cookie save on samesite

1ad85f9

fix: make email optional

0205a93

Merge branch 'develop' into feat/fe/auth-error-handling

64c6f1b

goniszewski approved these changes Oct 31, 2024

View reviewed changes

goniszewski merged commit 6eac509 into goniszewski:develop Oct 31, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: auth error handling #144

fix: auth error handling #144

Sparkenstein commented Oct 24, 2024

Sparkenstein commented Oct 24, 2024

Sparkenstein Oct 25, 2024

goniszewski Oct 25, 2024

Sparkenstein Oct 25, 2024

goniszewski Oct 25, 2024

goniszewski left a comment

goniszewski Oct 25, 2024

goniszewski Oct 25, 2024

goniszewski Oct 25, 2024

Sparkenstein Oct 27, 2024

goniszewski Oct 27, 2024

Sparkenstein Oct 27, 2024

goniszewski Oct 27, 2024

Sparkenstein Oct 27, 2024

Sparkenstein Oct 27, 2024

sonarcloud bot commented Oct 31, 2024

Sparkenstein commented Oct 31, 2024

goniszewski commented Oct 31, 2024

		@@ -2,26 +2,35 @@

		Thank you for considering contributing to this project! All contributors, big or small, are welcomed. To make the contribution process as smooth as possible, please follow the guidelines below.

		## Prerequisites

fix: auth error handling #144

fix: auth error handling #144

Conversation

Sparkenstein commented Oct 24, 2024

Sparkenstein commented Oct 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goniszewski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Oct 31, 2024

Quality Gate passed

Sparkenstein commented Oct 31, 2024

goniszewski commented Oct 31, 2024