From e969f7c3a7187b6e824b89b7b96d38ae89fe2aa1 Mon Sep 17 00:00:00 2001 From: Muhammad Fairuzi Teguh Date: Sun, 8 Mar 2020 11:15:37 +0700 Subject: [PATCH 1/6] [REFACTOR] Change modalLogin to modal-login --- frontend/src/components/{modalLogin.js => modal-login.js} | 0 frontend/src/components/navbar.js | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename frontend/src/components/{modalLogin.js => modal-login.js} (100%) diff --git a/frontend/src/components/modalLogin.js b/frontend/src/components/modal-login.js similarity index 100% rename from frontend/src/components/modalLogin.js rename to frontend/src/components/modal-login.js diff --git a/frontend/src/components/navbar.js b/frontend/src/components/navbar.js index 817be51..0585cc7 100644 --- a/frontend/src/components/navbar.js +++ b/frontend/src/components/navbar.js @@ -1,7 +1,7 @@ import React, { useState } from "react" import { Navbar as BNavbar, Nav, Button } from "react-bootstrap" import { Link as GatsbyLink } from "gatsby" -import ModalLogin from "./modalLogin" +import ModalLogin from "./modal-login" import "./navbar.scss" -- GitLab From dae43b36d8f67e72ab550585c281192b448d3b0f Mon Sep 17 00:00:00 2001 From: Muhammad Fairuzi Teguh Date: Sun, 8 Mar 2020 11:39:59 +0700 Subject: [PATCH 2/6] [RED] Add positive and negative tests for login --- frontend/package-lock.json | 5 +++ frontend/package.json | 1 + frontend/src/api.js | 7 +++ frontend/src/components/modal-login.js | 21 +++++++-- frontend/src/components/navbar.test.js | 62 ++++++++++++++++++++++---- 5 files changed, 85 insertions(+), 11 deletions(-) diff --git a/frontend/package-lock.json b/frontend/package-lock.json index b359623..282316a 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -16300,6 +16300,11 @@ "react-side-effect": "^1.1.0" } }, + "react-hook-form": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/react-hook-form/-/react-hook-form-5.0.1.tgz", + "integrity": "sha512-5Q6RvyN9vNm4wGard4Sw+xQz7rGIL+tS+UBVKFlTWcKhjsn87sKw2Uge6StxtcFUdUQyf+mNALNz3QRdHN3Pug==" + }, "react-hot-loader": { "version": "4.12.19", "resolved": "https://registry.npmjs.org/react-hot-loader/-/react-hot-loader-4.12.19.tgz", diff --git a/frontend/package.json b/frontend/package.json index 1e07b49..2f64384 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -33,6 +33,7 @@ "react-dom": "^16.12.0", "react-google-login": "^5.1.1", "react-helmet": "^5.2.1", + "react-hook-form": "^5.0.1", "redux": "^4.0.5" }, "devDependencies": { diff --git a/frontend/src/api.js b/frontend/src/api.js index 989a4b1..83acae5 100644 --- a/frontend/src/api.js +++ b/frontend/src/api.js @@ -3,3 +3,10 @@ import { BASE_API_URL } from "./config" export const getListJadwalDonor = date => axios.get(`${BASE_API_URL}/donor/jadwal/?date=${date}`) + +export const postUserAuth = (email, password) => + Promise.resolve({ + data: { + access: "initokenyangsecure", + }, + }) diff --git a/frontend/src/components/modal-login.js b/frontend/src/components/modal-login.js index a336f75..0ea3216 100644 --- a/frontend/src/components/modal-login.js +++ b/frontend/src/components/modal-login.js @@ -1,8 +1,11 @@ import React from "react" import { Modal, Button, Form } from "react-bootstrap" import { GoogleLogin } from "react-google-login" +import { useForm } from "react-hook-form" const ModalLogin = ({ show, handleClose }) => { + const { register, handleSubmit, errors } = useForm() + const onSubmit = data => console.log(data) return ( { Masuk -
+ - + - +
diff --git a/frontend/src/components/navbar.js b/frontend/src/components/navbar.js index 0585cc7..7584c08 100644 --- a/frontend/src/components/navbar.js +++ b/frontend/src/components/navbar.js @@ -4,12 +4,14 @@ import { Link as GatsbyLink } from "gatsby" import ModalLogin from "./modal-login" import "./navbar.scss" +import { useAuth } from "../hooks/authenticate" -const NavLink = ({ className, ...props }) => ( +const NavLink = ({ className = "", ...props }) => ( ) const Navbar = () => { + const { user, logout } = useAuth() const [showModalLogin, setShowModalLogin] = useState(false) return ( @@ -29,10 +31,19 @@ const Navbar = () => { Ajukan Acara Donor Profil - + {user ? ( + + ) : ( + + )} ({ __esModule: true, - postUserAuth: jest.fn(), + postUserLogin: jest.fn(), + postUserProfile: jest.fn(), })) +jest.mock("../hooks/authenticate.js", () => ({ + __esModule: true, + useAuth: jest.fn(), + AuthProvider: jest.fn(), +})) +useAuth.mockReturnValue({ + user: null, + login: () => {}, + register: () => {}, + logout: () => {}, +}) describe(`Navbar`, () => { it(`shows all the options`, () => { @@ -34,32 +47,61 @@ describe(`Login`, () => { }) it(`shows "Ubah Profil" to replace "Masuk" after login`, async () => { - postUserAuth.mockResolvedValueOnce({ + postUserLogin.mockResolvedValueOnce({ data: { access: "initokenyangsecure", }, }) + postUserProfile.mockResolvedValueOnce({ + data: { + email: "fairuzi@informatika.com", + nama: "Muhammad Fairuzi Teguh", + }, + }) + useAuth.mockImplementation( + require.requireActual("../hooks/authenticate").useAuth + ) + AuthProvider.mockImplementation( + require.requireActual("../hooks/authenticate").AuthProvider + ) - const { getByText, getByPlaceholderText, findByText } = render() + const { getByText, getByPlaceholderText, findByText, getByTestId } = render( + + + + ) - fireEvent.click(getByText("Masuk")) + fireEvent.click(getByText(/Masuk/i)) fireEvent.change(getByPlaceholderText(/Email/i), { target: { value: "muhammad.fairuzi@ui.ac.id" }, }) fireEvent.change(getByPlaceholderText(/Password/i), { target: { value: "passwordinirahasia" }, }) + fireEvent.click(getByTestId("button-login")) expect(await findByText(/Ubah Profil/i)).toBeInTheDocument() }) it(`shows error message when can't login`, async () => { - postUserAuth.mockRejectedValueOnce({ - data: { - detail: "passwordnyasalah", + postUserLogin.mockRejectedValueOnce({ + response: { + data: { + detail: "passwordnyasalah", + }, }, }) + useAuth.mockImplementation( + require.requireActual("../hooks/authenticate").useAuth + ) + AuthProvider.mockImplementation( + require.requireActual("../hooks/authenticate").AuthProvider + ) - const { getByText, getByPlaceholderText, findByText } = render() + const { getByText, getByPlaceholderText, findByText, getByTestId } = render( + + + + ) fireEvent.click(getByText("Masuk")) fireEvent.change(getByPlaceholderText(/Email/i), { @@ -68,6 +110,7 @@ describe(`Login`, () => { fireEvent.change(getByPlaceholderText(/Password/i), { target: { value: "passwordyangsalah" }, }) - expect(await findByText(/passwordnyasalah/i)).toBeInTheDocument() + fireEvent.click(getByTestId("button-login")) + expect(await findByText(/Email atau password salah./i)).toBeInTheDocument() }) }) diff --git a/frontend/src/hooks/authenticate.js b/frontend/src/hooks/authenticate.js new file mode 100644 index 0000000..19f41e0 --- /dev/null +++ b/frontend/src/hooks/authenticate.js @@ -0,0 +1,61 @@ +import React, { useEffect, useState } from "react" +import { postUserLogin, postUserProfile, postUserLogout } from "../api" + +const LOCAL_STORAGE_TOKEN_KEY = "token" + +const AuthContext = React.createContext() + +const AuthProvider = props => { + const [user, setUser] = useState(null) + + const postAndSetUserProfile = async token => { + try { + const { data: user } = await postUserProfile(token) + setUser(user) + } catch (error) { + // do nothing if the token is invalid + } + } + useEffect(() => { + const existentToken = window.localStorage.getItem(LOCAL_STORAGE_TOKEN_KEY) + if (existentToken) postAndSetUserProfile(existentToken) + }, []) + + const login = async (email, password) => { + // if login fail, it's ok to throw the error + const result = await postUserLogin(email, password) + const token = result.data.access + window.localStorage.setItem(LOCAL_STORAGE_TOKEN_KEY, token) + postAndSetUserProfile(token) + } + const register = () => {} + const logout = async () => { + // if not throwing error, than we are OK to proceed + await postUserLogout() + window.localStorage.removeItem(LOCAL_STORAGE_TOKEN_KEY) + setUser(null) + // TODO: axios + } + + return ( + + ) +} + +const useAuth = () => { + const context = React.useContext(AuthContext) + if (context === undefined) { + throw new Error("useAuth/useUser must be used within a AuthProvider") + } + return context +} + +const useUser = () => { + const { user } = useAuth() + return user +} + +export { AuthProvider, useAuth, useUser } diff --git a/frontend/src/styles/global.scss b/frontend/src/styles/global.scss index 2e80852..d476761 100644 --- a/frontend/src/styles/global.scss +++ b/frontend/src/styles/global.scss @@ -216,3 +216,8 @@ body { font-size: 17px; } } + +.invalid-feedback { + color: $red; + display: block; +} -- GitLab From 02965146b3f1758e3ca54edb3e72edbe706d9b5e Mon Sep 17 00:00:00 2001 From: Muhammad Fairuzi Teguh Date: Sun, 8 Mar 2020 19:12:13 +0700 Subject: [PATCH 4/6] [REFACTOR] Add more login scenarios --- frontend/src/components/modal-login.js | 4 +- frontend/src/components/navbar.test.js | 94 ++++++++++++++++++++------ frontend/src/hooks/authenticate.js | 12 ++-- 3 files changed, 79 insertions(+), 31 deletions(-) diff --git a/frontend/src/components/modal-login.js b/frontend/src/components/modal-login.js index 0b05c21..492366e 100644 --- a/frontend/src/components/modal-login.js +++ b/frontend/src/components/modal-login.js @@ -57,7 +57,7 @@ const ModalLogin = ({ show, handleClose }) => { /> {errors.email && ( - {errors.email.message || "Email tidak valid."} + {errors.email.message} )} @@ -71,7 +71,7 @@ const ModalLogin = ({ show, handleClose }) => { /> {errors.password && errors.password.type === "required" && ( - {errors.password.message || "Password tidak valid."} + {errors.password.message} )} diff --git a/frontend/src/components/navbar.test.js b/frontend/src/components/navbar.test.js index 5492d64..e2e1fad 100644 --- a/frontend/src/components/navbar.test.js +++ b/frontend/src/components/navbar.test.js @@ -1,18 +1,19 @@ -import React from "react" import { - render, fireEvent, + render, waitForElementToBeRemoved, + screen, } from "@testing-library/react" - +import React from "react" +import { postUserLogin, postUserProfile, postUserLogout } from "../api" +import { AuthProvider, useAuth } from "../hooks/authenticate" import Navbar from "./navbar" -import { postUserLogin, postUserProfile } from "../api" -import { useAuth, AuthProvider } from "../hooks/authenticate" jest.mock("../api.js", () => ({ __esModule: true, postUserLogin: jest.fn(), postUserProfile: jest.fn(), + postUserLogout: jest.fn(), })) jest.mock("../hooks/authenticate.js", () => ({ __esModule: true, @@ -46,6 +47,46 @@ describe(`Login`, () => { await waitForElementToBeRemoved(() => getByPlaceholderText("Email")) }) + it(`shows feedback when submit required field but empty`, async () => { + const { findByText, getByText, getByTestId } = render() + fireEvent.click(getByText(/Masuk/i)) + fireEvent.click(getByTestId("button-login")) + expect(await findByText(/Masukkan email/i)).toBeInTheDocument() + expect(await findByText(/Masukkan Password/i)).toBeInTheDocument() + }) + + it(`shows error message when can't login`, async () => { + postUserLogin.mockRejectedValueOnce({ + response: { + data: { + detail: "passwordnyasalah", + }, + }, + }) + useAuth.mockImplementation( + require.requireActual("../hooks/authenticate").useAuth + ) + AuthProvider.mockImplementation( + require.requireActual("../hooks/authenticate").AuthProvider + ) + + const { getByText, getByPlaceholderText, findByText, getByTestId } = render( + + + + ) + + fireEvent.click(getByText("Masuk")) + fireEvent.change(getByPlaceholderText(/Email/i), { + target: { value: "muhammad.fairuzi@ui.ac.id" }, + }) + fireEvent.change(getByPlaceholderText(/Password/i), { + target: { value: "passwordyangsalah" }, + }) + fireEvent.click(getByTestId("button-login")) + expect(await findByText(/Email atau password salah./i)).toBeInTheDocument() + }) + it(`shows "Ubah Profil" to replace "Masuk" after login`, async () => { postUserLogin.mockResolvedValueOnce({ data: { @@ -64,8 +105,7 @@ describe(`Login`, () => { AuthProvider.mockImplementation( require.requireActual("../hooks/authenticate").AuthProvider ) - - const { getByText, getByPlaceholderText, findByText, getByTestId } = render( + const { getByText, getByPlaceholderText, getByTestId } = render( @@ -78,16 +118,23 @@ describe(`Login`, () => { fireEvent.change(getByPlaceholderText(/Password/i), { target: { value: "passwordinirahasia" }, }) - fireEvent.click(getByTestId("button-login")) - expect(await findByText(/Ubah Profil/i)).toBeInTheDocument() + fireEvent.click(screen.getByTestId("button-login")) + await waitForElementToBeRemoved(() => + screen.getByText(/Belum punya akun?/i) + ) + expect(await screen.findByText(/Ubah Profil/i)).toBeInTheDocument() }) - it(`shows error message when can't login`, async () => { - postUserLogin.mockRejectedValueOnce({ - response: { - data: { - detail: "passwordnyasalah", - }, + it(`shows "Masuk" again after logout`, async () => { + postUserLogin.mockResolvedValueOnce({ + data: { + access: "initokenyangsecure", + }, + }) + postUserProfile.mockResolvedValueOnce({ + data: { + email: "fairuzi@informatika.com", + nama: "Muhammad Fairuzi Teguh", }, }) useAuth.mockImplementation( @@ -96,21 +143,26 @@ describe(`Login`, () => { AuthProvider.mockImplementation( require.requireActual("../hooks/authenticate").AuthProvider ) - - const { getByText, getByPlaceholderText, findByText, getByTestId } = render( + const { getByText, getByPlaceholderText } = render( ) - fireEvent.click(getByText("Masuk")) + fireEvent.click(getByText(/Masuk/i)) fireEvent.change(getByPlaceholderText(/Email/i), { target: { value: "muhammad.fairuzi@ui.ac.id" }, }) fireEvent.change(getByPlaceholderText(/Password/i), { - target: { value: "passwordyangsalah" }, + target: { value: "passwordinirahasia" }, }) - fireEvent.click(getByTestId("button-login")) - expect(await findByText(/Email atau password salah./i)).toBeInTheDocument() + fireEvent.click(screen.getByTestId("button-login")) + await waitForElementToBeRemoved(() => + screen.getByText(/Belum punya akun?/i) + ) + + postUserLogout.mockResolvedValueOnce({ data: { detail: "SUCCESS" } }) + fireEvent.click(await screen.findByText(/Keluar/i)) + expect(await screen.findByText(/Masuk/i)).toBeInTheDocument() }) }) diff --git a/frontend/src/hooks/authenticate.js b/frontend/src/hooks/authenticate.js index 19f41e0..93d462c 100644 --- a/frontend/src/hooks/authenticate.js +++ b/frontend/src/hooks/authenticate.js @@ -27,6 +27,7 @@ const AuthProvider = props => { const token = result.data.access window.localStorage.setItem(LOCAL_STORAGE_TOKEN_KEY, token) postAndSetUserProfile(token) + // TODO: axios set token } const register = () => {} const logout = async () => { @@ -34,7 +35,7 @@ const AuthProvider = props => { await postUserLogout() window.localStorage.removeItem(LOCAL_STORAGE_TOKEN_KEY) setUser(null) - // TODO: axios + // TODO: axios set token } return ( @@ -48,14 +49,9 @@ const AuthProvider = props => { const useAuth = () => { const context = React.useContext(AuthContext) if (context === undefined) { - throw new Error("useAuth/useUser must be used within a AuthProvider") + throw new Error("useAuth must be used within a AuthProvider") } return context } -const useUser = () => { - const { user } = useAuth() - return user -} - -export { AuthProvider, useAuth, useUser } +export { AuthProvider, useAuth } -- GitLab From b2c9ce7f8ea34ac7cf89534059a76b5a0957bb3f Mon Sep 17 00:00:00 2001 From: Muhammad Fairuzi Teguh Date: Sun, 8 Mar 2020 19:19:14 +0700 Subject: [PATCH 5/6] [REFACTOR] Separate successful login method to avoid code duplication --- frontend/src/components/navbar.test.js | 106 +++++++++---------------- 1 file changed, 37 insertions(+), 69 deletions(-) diff --git a/frontend/src/components/navbar.test.js b/frontend/src/components/navbar.test.js index e2e1fad..63439d6 100644 --- a/frontend/src/components/navbar.test.js +++ b/frontend/src/components/navbar.test.js @@ -35,6 +35,41 @@ describe(`Navbar`, () => { }) }) +const doSuccessfulLogin = async () => { + postUserLogin.mockResolvedValueOnce({ + data: { + access: "initokenyangsecure", + }, + }) + postUserProfile.mockResolvedValueOnce({ + data: { + email: "fairuzi@informatika.com", + nama: "Muhammad Fairuzi Teguh", + }, + }) + useAuth.mockImplementation( + require.requireActual("../hooks/authenticate").useAuth + ) + AuthProvider.mockImplementation( + require.requireActual("../hooks/authenticate").AuthProvider + ) + const { getByText, getByPlaceholderText } = render( + + + + ) + + fireEvent.click(getByText(/Masuk/i)) + fireEvent.change(getByPlaceholderText(/Email/i), { + target: { value: "muhammad.fairuzi@ui.ac.id" }, + }) + fireEvent.change(getByPlaceholderText(/Password/i), { + target: { value: "passwordinirahasia" }, + }) + fireEvent.click(screen.getByTestId("button-login")) + await waitForElementToBeRemoved(() => screen.getByText(/Belum punya akun?/i)) +} + describe(`Login`, () => { it(`shows "Masuk" button that can be clicked`, async () => { const { getByText, getByPlaceholderText } = render() @@ -88,79 +123,12 @@ describe(`Login`, () => { }) it(`shows "Ubah Profil" to replace "Masuk" after login`, async () => { - postUserLogin.mockResolvedValueOnce({ - data: { - access: "initokenyangsecure", - }, - }) - postUserProfile.mockResolvedValueOnce({ - data: { - email: "fairuzi@informatika.com", - nama: "Muhammad Fairuzi Teguh", - }, - }) - useAuth.mockImplementation( - require.requireActual("../hooks/authenticate").useAuth - ) - AuthProvider.mockImplementation( - require.requireActual("../hooks/authenticate").AuthProvider - ) - const { getByText, getByPlaceholderText, getByTestId } = render( - - - - ) - - fireEvent.click(getByText(/Masuk/i)) - fireEvent.change(getByPlaceholderText(/Email/i), { - target: { value: "muhammad.fairuzi@ui.ac.id" }, - }) - fireEvent.change(getByPlaceholderText(/Password/i), { - target: { value: "passwordinirahasia" }, - }) - fireEvent.click(screen.getByTestId("button-login")) - await waitForElementToBeRemoved(() => - screen.getByText(/Belum punya akun?/i) - ) + await doSuccessfulLogin() expect(await screen.findByText(/Ubah Profil/i)).toBeInTheDocument() }) it(`shows "Masuk" again after logout`, async () => { - postUserLogin.mockResolvedValueOnce({ - data: { - access: "initokenyangsecure", - }, - }) - postUserProfile.mockResolvedValueOnce({ - data: { - email: "fairuzi@informatika.com", - nama: "Muhammad Fairuzi Teguh", - }, - }) - useAuth.mockImplementation( - require.requireActual("../hooks/authenticate").useAuth - ) - AuthProvider.mockImplementation( - require.requireActual("../hooks/authenticate").AuthProvider - ) - const { getByText, getByPlaceholderText } = render( - - - - ) - - fireEvent.click(getByText(/Masuk/i)) - fireEvent.change(getByPlaceholderText(/Email/i), { - target: { value: "muhammad.fairuzi@ui.ac.id" }, - }) - fireEvent.change(getByPlaceholderText(/Password/i), { - target: { value: "passwordinirahasia" }, - }) - fireEvent.click(screen.getByTestId("button-login")) - await waitForElementToBeRemoved(() => - screen.getByText(/Belum punya akun?/i) - ) - + await doSuccessfulLogin() postUserLogout.mockResolvedValueOnce({ data: { detail: "SUCCESS" } }) fireEvent.click(await screen.findByText(/Keluar/i)) expect(await screen.findByText(/Masuk/i)).toBeInTheDocument() -- GitLab From b72c4d79faa8f742eef844d9a9cee2cf18403e91 Mon Sep 17 00:00:00 2001 From: Muhammad Fairuzi Teguh Date: Sun, 8 Mar 2020 19:39:20 +0700 Subject: [PATCH 6/6] [REFACTOR] Add test for useAuth to throw when called outside its provider --- frontend/src/hooks/authenticate.test.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 frontend/src/hooks/authenticate.test.js diff --git a/frontend/src/hooks/authenticate.test.js b/frontend/src/hooks/authenticate.test.js new file mode 100644 index 0000000..d689cde --- /dev/null +++ b/frontend/src/hooks/authenticate.test.js @@ -0,0 +1,23 @@ +import React from "react" +import { useAuth } from "./authenticate" +import { render } from "@testing-library/react" + +describe(`useAuth`, () => { + it(`throws error when used outside of the provider`, () => { + // React will call console.error for this expected error + // mock to make output more concise + jest.spyOn(console, "error").mockImplementation((...args) => {}) + const OuterComponent = () => { + return ( + <> + + + ) + } + const UsingUseAuthOutsideProvider = () => { + useAuth() + return <> + } + expect(() => render()).toThrow() + }) +}) -- GitLab