Andras/BoundlessFlowCampus2K
4
7
Fork 0
Code Issues 90 Pull requests 7 Projects Releases Packages Wiki Activity Actions

1108-login-error-messages #1114

Merged
a22erigr merged 10 commits from 1108-login-error-messages into team_2_week_7 2026-05-26 08:27:10 +00:00
Conversation 10 Commits 10 Files changed 6 +243 -194
b23albst commented 2026-05-22 18:20:30 +00:00
Collaborator
Copy link

Whats's done

  • service/loginService/loginService/Services/AuthService.cs
    • The error type when invalid credentials are used has been changed so that every error in AuthenticateAsync have unique types.
  • frontend/src/features/login/useLogin.js and frontend/src/features/login/useSignup.js
  • Instead of throwing errors with the text returned from the post as message, the message is just the http status code. Also the identification of too short passwords and invalid emails have been moved from LoginModal.js to useSignup.js. When these occur, errors are thrown.
  • frontend/src/features/login/LoginModal.js
    • Instead of just displaying the thrown error's message, now gets printed error message from localization.
  • frontend/src/locales/en.json and frontend/src/locales/sv.json
    • Error messages have been added. Also, after a merge the files had to be somewhat reordered again.
# Whats's done + service/loginService/loginService/Services/AuthService.cs + The error type when invalid credentials are used has been changed so that every error in AuthenticateAsync have unique types. + frontend/src/features/login/useLogin.js and frontend/src/features/login/useSignup.js + Instead of throwing errors with the text returned from the post as message, the message is just the http status code. Also the identification of too short passwords and invalid emails have been moved from LoginModal.js to useSignup.js. When these occur, errors are thrown. + frontend/src/features/login/LoginModal.js + Instead of just displaying the thrown error's message, now gets printed error message from localization. + frontend/src/locales/en.json and frontend/src/locales/sv.json + Error messages have been added. Also, after a merge the files had to be somewhat reordered again.
c24elipe reviewed 2026-05-25 06:54:40 +00:00
c24elipe left a comment
Collaborator
Copy link

At first glance, all looks good. I will now look a bit closer.

At first glance, all looks good. I will now look a bit closer.
c24elipe commented 2026-05-25 07:08:54 +00:00
Collaborator
Copy link

image
image
image
image
image

![image](/attachments/0e50e1cc-1d82-4ccf-81a0-c092f50dc80c) ![image](/attachments/75490638-75f1-4112-8747-22a11b1be090) ![image](/attachments/f22e4d0a-85f4-4109-bb7d-2d08a5ee4363) ![image](/attachments/d3903cfa-0d5d-4102-b4e4-9cfae462f4c9) ![image](/attachments/4959865d-e396-4ad6-9720-a05a3ebc42ab)
image.png
37 KiB
image.png
38 KiB
image.png
36 KiB
image.png
35 KiB
image.png
38 KiB
c24elipe commented 2026-05-25 07:11:00 +00:00
Collaborator
Copy link

Two quick improvement areas found:

  1. When I close the modal and open it again, the inputs remain. It might be better if they get cleared?
  2. One must press the button. A quality of life thing would be to make the enter key work to submit.
Two quick improvement areas found: 1. When I close the modal and open it again, the inputs remain. It might be better if they get cleared? 2. One must press the button. A quality of life thing would be to make the enter key work to submit.
a22erigr commented 2026-05-25 07:36:22 +00:00
Collaborator
Copy link

@b23albst Can you implement the changes before i merge?

@b23albst Can you implement the changes before i merge?
c24elipe commented 2026-05-25 07:47:22 +00:00
Collaborator
Copy link

Review on Pr #1114, Issue #1108

Reviewed the code and the visual elements when used.

Tested on Arch Linux using Librewolf (a Firefox browser)

  • As the images above suggest, the app starts.

Inspected the following files

  • Indentation/code styling
    • All files follow the prettier standard.
  • Naming convention
    • Some problems are here. Mainly, that shorthand for variable names is used.
      • useSignup.js, lines 33, 35, 37 all use res instead of results
      • useLogin.js, same here, lines 23, 27, 30 all use res instead of results
      • LoginModal.js: On line 66, err is used instead of error.
  • Comments
    • At some points (like around the regex), it is a bit hard to read. But the meaning comes across without problems... so Okey
  • General code quality
    • As far as I can tell, it's clean as it can be. Normally, I would blame it for not using useApi, but due to the difficulty, I say that it is okay.

What needs to be done

  1. Maybe add a comment or so more, so the next reader can understand the regex in useSignUp line 16->17. But this is a petty and small request.

  2. Just remove the shorthand names.

  3. Add so the modlars are reset when the values are submitted. (But that is optional.)

Conclusion

This shouldn't be merged.

# Review on Pr #1114, Issue #1108 Reviewed the code and the visual elements when used. ### Tested on Arch Linux using Librewolf (a Firefox browser) * As the images above suggest, the app starts. ### Inspected the following files * **Indentation/code styling** * All files follow the prettier standard. * **Naming convention** * Some problems are here. Mainly, that shorthand for variable names is used. * **useSignup.js**, lines 33, 35, 37 all use res instead of results * **useLogin.js**, same here, lines 23, 27, 30 all use res instead of results * **LoginModal.js**: On line 66, err is used instead of error. * **Comments** * At some points (like around the regex), it is a bit hard to read. But the meaning comes across without problems... so Okey * **General code quality** * As far as I can tell, it's clean as it can be. Normally, I would blame it for not using useApi, but due to the difficulty, I say that it is okay. ### What needs to be done 1. Maybe add a comment or so more, so the next reader can understand the regex in useSignUp line 16->17. But this is a petty and small request. 2. Just remove the shorthand names. 3. Add so the modlars are reset when the values are submitted. (But that is optional.) ### Conclusion This shouldn't be merged.
b23albst commented 2026-05-25 11:10:31 +00:00
Author
Collaborator
Copy link

I have now implemented the requested changes. Something to be noted is that I rearranged the localization keys when I started working on this, since they were not in the proper order. If changes have been made to them since then, these might be all mixed up again since I merged team_2_week_7 into this branch to solve the merge conflicts...

I have now implemented the requested changes. Something to be noted is that I rearranged the localization keys when I started working on this, since they were not in the proper order. If changes have been made to them since then, these might be all mixed up again since I merged team_2_week_7 into this branch to solve the merge conflicts...
👍 1
j21marbr commented 2026-05-26 07:48:46 +00:00
Collaborator
Copy link

Review on #1114

Whats done

Tested on macOS using Chrome

  • App starts
    • Sign up functionality works
    • Login functionality works
    • Enter key submits login/sign up
    • Modal fields reset correctly when closing

Inspected following files

  • LoginModal.js, useLogin.js, useSignup.js
    • Indentation/code styling
      • Follows standard
    • Naming convention
      • Previously mentioned shorthand names have been removed
    • Comments
      • Regex comment added and looks clear
    • General code quality
      • No issues found

What needs to be done

  • Invalid login attempts currently display the "Enter both username and password" message even when both fields are filled

Conclusion

  • The requested changes have been implemented correctly but the invalid login error handling should be fixed before merge.
# Review on #1114 ## Whats done ### Tested on macOS using Chrome + App starts + Sign up functionality works + Login functionality works + Enter key submits login/sign up + Modal fields reset correctly when closing ### Inspected following files + LoginModal.js, useLogin.js, useSignup.js + Indentation/code styling + Follows standard + Naming convention + Previously mentioned shorthand names have been removed + Comments + Regex comment added and looks clear + General code quality + No issues found ## What needs to be done + Invalid login attempts currently display the "Enter both username and password" message even when both fields are filled ## Conclusion + The requested changes have been implemented correctly but the invalid login error handling should be fixed before merge.
j21marbr commented 2026-05-26 07:49:07 +00:00
Collaborator
Copy link
No description provided.
bild.png
110 KiB
bild.png
j21marbr changed title from 1108-login-error-messages to WIP: 1108-login-error-messages 2026-05-26 08:00:03 +00:00
j21marbr changed title from WIP: 1108-login-error-messages to 1108-login-error-messages 2026-05-26 08:00:14 +00:00
b23albst commented 2026-05-26 08:08:54 +00:00
Author
Collaborator
Copy link

This error does not occur for me. Did you start with npm start?
image
image

This error does not occur for me. Did you start with npm start? ![image](/attachments/f8fac598-70dc-48f4-a6f3-41a60f43a803) ![image](/attachments/e613b179-48c1-4c5e-8d40-683d40614f31)
image.png
38 KiB
image.png
38 KiB
j21marbr commented 2026-05-26 08:24:15 +00:00
Collaborator
Copy link

I tested again and it worked, the code can be merged

I tested again and it worked, the code can be merged
👍 1
a22erigr merged commit a797e99b5c into team_2_week_7 2026-05-26 08:27:10 +00:00
a22erigr deleted branch 1108-login-error-messages 2026-05-26 08:27:11 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
c24elipe
j21marbr
Labels
Clear labels   
_CRITICAL_

Only use this label for critical tasks that are project-scope,and must be addressed ASAP.

  
API

   
app

   
backEnd

   
Blocked, waiting for further changes

   
bug

Something is not working

  
cleanup

Issues to cleanup the codebase (restructure folders, remove deprecated or smelly code)

  
close

Issue to close

  
design

   
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
feature request

   
frontEnd

   
help wanted

Need some help

  
invalid

Something is wrong

  
low priority

   
needs input

Issue needs input from product owner or customer

  
needs review

issue is in need of review

  
project documentation

   
question

More information is needed

  
research

   
reviewed

This Issue has been reviewed

  
script

   
security

use this for security-related issues

  
SQL

  
style

   
testing

   
topLevel

Reserved for top-level, project-wide issues.

  
wontfix

This won't be fixed

No labels
_CRITICAL_
API
app
backEnd
Blocked, waiting for further changes
bug
cleanup
close
design
duplicate
enhancement
feature request
frontEnd
help wanted
invalid
low priority
needs input
needs review
project documentation
question
research
reviewed
script
security
SQL
style
testing
topLevel
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Andras/BoundlessFlowCampus2K!1114
No description provided.