This pull request is mainly for @superpowers11 but @fallcat, you can review it too if you would like 👍
154
🎉 One more month before this gets fully used 🎉
As usual please don't be intimidated by the GitHub-produced diffs. They exaggerate the amount of changes I made.
This pull request focuses on removing the return null expressions from the LoginService. I replace them with throwing Exceptions.
I think this is a good idea because:
This explicitly lets faculty/non-reslife staff know upfront that they can't access the system instead of showing them an ugly error page :/
Throwing an exception with a message tells the developer exactly what failed, where and why. This is in contrast to returning null which will eventually throw an unhelpful ArgumentNullException down the line if something went wrong.
Flattens logic i.e. No more nested if/else in the LoginController
Testing:
Currently writing UI tests for this. I will be testing login attempts with valid, invalid, missing and staff credentials (e.g. 360.facultytest; lol I forgot this existed 🐐 )
I have manually tested this by:
Successfully login in as myself.
Getting an error message when I try to log in with no credentials
Getting an error message when I try to log in with incorrect credentials.
Getting an error message when I try to log in as 360.facultytest
This pull request is mainly for @superpowers11 but @fallcat, you can review it too if you would like 👍
154
🎉 One more month before this gets fully used 🎉
As usual please don't be intimidated by the GitHub-produced diffs. They exaggerate the amount of changes I made.
This pull request focuses on removing the
return nullexpressions from the LoginService. I replace them with throwing Exceptions.I think this is a good idea because:
ArgumentNullExceptiondown the line if something went wrong.LoginControllerTesting: