Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#171380695 user login #12

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Ugizwenayo-Divine
Copy link
Collaborator

@Ugizwenayo-Divine Ugizwenayo-Divine commented May 27, 2020

What does this PR do?

  • user should be able to login

Description of Task to be completed?

  • Install required libraries
  • Create UserLogin component

How should this be manually tested?

  • In your terminal, run git clone https://github.com/Stackup-Rwanda/stackup2-barefoot-frontend.git to clone this repo

  • Run `git fetch origin ft-user-login-171380695

  • Run git checkout ft-user-login-171380695 to switch to the fetched branch above

  • Run yarn install to install all dependencies

  • Start the development server by running yarn dev.

  • The server should launch the app in your default browser at localhost:{{PORT}}

  • Click the login button, you will see the login form

  • try to login with the user in database

Any background context you want to provide?

N/A

What are the relevant pivotal tracker stories?

#171380695

@Ugizwenayo-Divine Ugizwenayo-Divine added the WIP Work In Progress label May 27, 2020
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam May 27, 2020 11:24 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 3, 2020 13:07 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 4, 2020 14:37 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 4, 2020 15:20 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 4, 2020 16:47 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 4, 2020 20:28 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine added Ready for review Ready for peer review and removed WIP Work In Progress labels Jun 4, 2020
@Ugizwenayo-Divine Ugizwenayo-Divine changed the title (feat): userLogin #171380695 user login Jun 5, 2020
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 5, 2020 12:37 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 8, 2020 13:30 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 9, 2020 13:55 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 10, 2020 08:03 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 10, 2020 12:40 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 10, 2020 12:47 Inactive

const login = async (user) => {
let result = '';
const url = 'https://bft-nmd-stag.herokuapp.com/api/auth/login';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this URL to the .env file as it is a variable that is better handled there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've implemented this feedback

const loginError = (data) => ({ type: LOGIN_HANDLE, payload: data });

const handleLogin = (credentials) => async (dispatch) => {
let output = '';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to declare this variable here. Delete this line and on line 15 do const output = token ...

},
});

const InputField = (props) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could destructure props directly here instead of line 22.
const InputField = ({ handleLoginChange, name }) => {}

@Ugizwenayo-Divine
Copy link
Collaborator Author

Kudos! @Ugizwenayo-Divine
Could you take care of the following:

  1. I followed the instructions to test this PR and when I run yarn dev I get a page with a fullscreen image, nothing else. Is this the intended scenario?

Screenshot from 2020-06-10 18-47-05

  1. Could you make sure to remove all the commented codes?
  2. For redux, I highly recommend you do this: Create a redux folder then inside you create a rootReducer file that will combine all the reducers
  3. In redux folder, create a login folder and inside it put the actions, reducer and types for login.
    This way it will be easier to maintain as the app scales and we have a lot of reducers and actions.
  4. Rebase

there should be a login button to click because for me it's there

@the22mastermind
Copy link
Collaborator

Kudos! @Ugizwenayo-Divine
Could you take care of the following:

  1. I followed the instructions to test this PR and when I run yarn dev I get a page with a fullscreen image, nothing else. Is this the intended scenario?

Screenshot from 2020-06-10 18-47-05

  1. Could you make sure to remove all the commented codes?
  2. For redux, I highly recommend you do this: Create a redux folder then inside you create a rootReducer file that will combine all the reducers
  3. In redux folder, create a login folder and inside it put the actions, reducer and types for login.
    This way it will be easier to maintain as the app scales and we have a lot of reducers and actions.
  4. Rebase

there should be a login button to click because for me it's there

Could you share a screenshot? I even tried inspecting the HTML and I saw only the main div with an id of root

@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 11, 2020 09:10 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 11, 2020 10:58 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 11, 2020 11:19 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 12, 2020 07:53 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 12, 2020 08:01 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 12, 2020 08:08 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 12, 2020 08:14 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 12, 2020 08:31 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 12, 2020 08:34 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 12, 2020 08:37 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine force-pushed the ft-user-login-171380695 branch 2 times, most recently from 3852505 to 8fa2bdc Compare June 12, 2020 08:49
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 12, 2020 08:49 Inactive
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 12, 2020 08:59 Inactive
- user should be able to login
- user should receive appropriate error message

[Finishes #171380695]
@Ugizwenayo-Divine Ugizwenayo-Divine temporarily deployed to stackup-pipe-ft-user-lo-uf8nam June 12, 2020 09:07 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready for review Ready for peer review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants