Feat: Login UI Issue#14 #26

mtreacy002 · 2020-06-20T07:30:29Z

Description

Add Login UI

Fixes #14 and #29

Type of Change:

Code
User Interface

Code/Quality Assurance Only

New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

running npm start and submit without input. Form validation required gave warning as expected.
login with existing user but with wrong password or username, error message is shown as expected.
login with correct credentials, user redirected to Member Portfolio page as expected.

link to Gif

Note to tester/reviewer. To test this login ui you must run the backend server from the latest code change on login backend PR#60

Checklist:

My PR follows the style guidelines of this project
I have performed a self-review of my own code or materials

Code/Quality Assurance Only

My changes generate no new warnings

Additional Note
Up to User Interface only, backend API consumption is not yet added - wip

mtreacy002 · 2020-06-20T07:32:46Z

Update @anitab-org/bridgeintech-maintainers , Just pushed this Login form as user interface. Form validation required has been applied but backend API consumption is WIP.

mtreacy002 · 2020-06-22T02:23:51Z

Update @anitab-org/bridgeintech-maintainers . This form now has error responses shown to user. I still need to workout how to persist the jwt token that got returned and carry that to the next page. @meenakshi-dhanani if you can give me some guidance in this, that'll be great. 😉

meenakshi-dhanani · 2020-06-24T15:46:43Z

Try and have more meaningful commit messages which indicate intent. eg. Allow a user to login.

src/Routes.js

src/login/Login.css

src/login/Login.jsx

meenakshi-dhanani · 2020-06-24T17:12:54Z

src/login/Login.jsx

+
+        new FormData(e.target).forEach((value, key) => {
+            if (key === "username")
+                userInput[key] = value;


Do you really need an if else statement here? The action that you are performing is the same either ways.
Why can't it simply be userInput[key] = value?

src/login/Login.jsx

meenakshi-dhanani · 2020-06-24T17:15:55Z

src/login/Login.jsx

+                                    type="text"
+                                    name="username"
+                                    placeholder="Username or Email"
+                                    onChange={e => setUsername(e.target.value)}


there is no need to set here. You are fetching the values from FormData anyway. So even if you set here, it's not being used

done. remove onChange

src/login/Login.jsx

src/tests/Login.test.jsx

mtreacy002 · 2020-06-25T00:42:34Z

Update @anitab-org/bridgeintech-maintainers and @meenakshi-dhanani . I've just pushed the changes requested in your review. Can you please re-review? Thanks

mtreacy002 · 2020-06-25T00:58:41Z

Posted this in a wrong PR 🤣

update @anitab-org/bridgeintech-maintainers . Successful logged in user is now redirected to Member Portfolio page.

link to GIF

mtreacy002 · 2020-06-25T04:48:37Z

@meenakshi-dhanani and @anitab-org/bridgeintech-maintainers . I've pull rebased the latest merged to develop and fixed few things mentioned by Meena on the feedback;

changed switch to if else,
removed unused useState (access token and access expiry)
redo setting payload logic

PS: While I'm on it, I did the same in Register.js (changed setting up the payload logic and removed unused useState) for consistency.

meenakshi-dhanani · 2020-06-25T04:52:26Z

src/login/Login.jsx

+
+
+export default function Login() {
+    const [errorMessage, setResponseMessage] = useState(null);


errorMessage, setErrorMessage - try following the convention

yep, missed that 😁

src/tests/Login.test.jsx

meenakshi-dhanani

I have tested the following scenarios:

Redirect to My Space on successful login
Show error message when server is down
Show appropriate error for invalid credentials (401)

The unit tests for these also have passed. Approving!

meenakshi-dhanani · 2020-06-26T14:10:52Z

@foongminwong could you test these? I've done a code review for Maya. I'm hoping you can add gifs and screenshots 😄

foongminwong

The changes made in this PR were tested locally. Following are the results:

Code review - Done by @meenakshi-dhanani
All possible responses were tested as below (tap/zoom in to see clearer gifs):

Login API	Login UI	Expected Result	Actual Result
		As a user, when I login with incorrect username/password, it should show an error.	✅
		As a user, when I login with correct credentials, it should direct me to a BIT success landing page.	✅
		As a user, if I leave one of the fields blank, it should prompt me to enter values for username/password.	✅
		As a user, when I login with an unverified account, it should tell me verify my account first before logging in.	✅

Additional testcases covered: N/A
Additional Comments:

Great job @mtreacy002 !! Merge Time ⏲

OS Version: Windows 10

mtreacy002 · 2020-06-27T03:32:44Z

Wowww....That's an amazing tests report, @foongminwong. Thank youuu... 😍

mtreacy002 self-assigned this Jun 20, 2020

mtreacy002 added Category: Coding Changes to code base or refactored code that doesn't fix a bug. Category: User Interface Improvements or additions to design. Program: GSOC Related to work completed during the Google Summer of Code Program. labels Jun 20, 2020

mtreacy002 added this to the GSoC: Coding Phase I milestone Jun 20, 2020

mtreacy002 requested a review from a team June 22, 2020 02:21

mtreacy002 force-pushed the issue14-login-ui branch 6 times, most recently from bea2e8f to 4913271 Compare June 23, 2020 08:54