feature(users): user registration #13

MuhanguziDavid · 2018-11-01T21:14:33Z

What does this PR do?

It enables users to signup using Authors Heaven front end.

Description of tasks to be completed?

Currently, a user cannot signup to Authors Heaven using the front end
This PR adds functionality to allow users to signup to Authors Heaven using the front end

How should this be manually tested?

Run npm install to install all dependencies
Run npm start
Enter the url http://localhost:3000/signup in your browser.
Enter user details in the registration form and click submit

What are the relevant pivotal tracker stories?

#160609513

Any background context you want to add?

N/A

Screenshots

RonKbS · 2018-11-02T04:16:40Z

src/actions/registerUserActions.js

+import { toast } from 'react-toastify';
+import { REGISTER_USER_SUCCESS, REGISTER_USER_ERROR } from './types';
+
+export const fetchUsers = (postData) => dispatch => {


Nice work on implementing this feature, @MuhanguziDavid.
This block of code is unclear to me. Is it necessary to have two arrow functions like this?

@RonKbS , yes, it is necessary. Like having one function within another.

RonKbS · 2018-11-02T04:21:53Z

The circleci and travis tests are failing, @MuhanguziDavid.
It may be because you have not included some packages in the package.json file.

coveralls · 2018-11-03T21:00:31Z

Pull Request Test Coverage Report for Build 129

39 of 39 (100.0%) changed or added relevant lines in 5 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-1.5%) to 98.507%

Totals
Change from base Build 95:	-1.5%
Covered Lines:	57
Relevant Lines:	57

💛 - Coveralls

mendozabree · 2018-11-04T20:15:06Z

src/actions/registerUserActions.js

+    .then((response) => {
+      localStorage.setItem('auth_token', response.data.user.auth_token);
+      dispatch({ type: REGISTER_USER_SUCCESS, payload: true });
+      toast.success('Signup successful', { autoClose: false });


Good work @MuhanguziDavid! I am impressed with your test coverage!
You should consider changing the autoClose functionality.
Something of the sort { autoClose: 3500, hideProgressBar: true } the latter part removes the default progress bar. The same toast change applies to the error toast on line 33.

The autoClose functionality has been changed

mendozabree

@MuhanguziDavid, good work with testing the component and the naming conventions.
Though you missed one, consider renaming the component test file, for example, a component named CreateArticle.js would have its corresponding file named CreateArticle.test.js

dondrzzy · 2018-11-05T06:09:02Z

src/actions/registerUserActions.js

+      toast.success('Signup successful', { autoClose: false });
+    })
+    .catch((error) => {
+      let emailKey = 'Email: ';


Please use a for-in loop to check the errors object. Link

dondrzzy · 2018-11-05T06:31:16Z

src/components/register/RegisterUser.js

+    const { email, password, username } = this.state;
+    return (
+      <div className="registerForm">
+        <h1>Register</h1>


Get a few ideas from here

dondrzzy · 2018-11-05T06:31:43Z

src/components/register/RegisterUser.js

+
+const mapStateToProps = state => ({
+  registerUserSuccess: state.user.registerUserSuccess,
+  registerUserError: state.user.registerUserError,


Do you still need this?

I left that there in the event that errors are shown within the form

dondrzzy · 2018-11-05T06:35:19Z

src/actions/registerUserActions.js

+import { REGISTER_USER_SUCCESS, REGISTER_USER_ERROR } from './types';
+
+export const fetchUsers = (postData) => dispatch => {
+  axios


Please use the axios Instance here. Link

dondrzzy · 2018-11-05T06:38:12Z

src/tests/actions/registerUserAction.test.js

+  const passwordKey = 'Password: ';
+
+  beforeEach(() => {
+    mockAdapter = new MockAdapter(axios);


Use mock instead of mockAdapter

dondrzzy · 2018-11-05T06:39:13Z

src/tests/actions/registerUserAction.test.js

+    store = mockStore({});
+  });
+
+  it('should register a user', async () => {


This test should be in the directory tests/actions/userActions.

dondrzzy · 2018-11-05T06:41:00Z

src/reducers/registerUserReducer.js

@@ -0,0 +1,25 @@
+import { REGISTER_USER_SUCCESS, REGISTER_USER_ERROR } from '../actions/types';


userReducer not registerUserReducer

dondrzzy · 2018-11-05T06:41:24Z

src/actions/registerUserActions.js

@@ -0,0 +1,35 @@
+import axios from 'axios';


userActions not registerUserActions

dondrzzy · 2018-11-05T06:45:14Z

src/tests/actions/registerUserAction.test.js

+  it('should not register a user without a valid username', async () => {
+    const response_data = {
+      token: 'my secret token',
+      errors: { email: ['This field may not be blank.'], password: ['This field may not be blank.'], username: null },


Email value should never be an array of a string. Get the 0 index value.

dondrzzy · 2018-11-05T06:46:33Z

src/tests/components/registerUser.test.js

+    expect(fetchUsers.mock.calls[2][0]).toEqual({ user: { email: 'user100@gmail.com', password: 'Abc12345', username: 'user100' } });
+  });
+
+  it('should redirect on successful login', () => {


What is this test doing?

I had put it there to test whether there is redirection on successful user signup, the test has now been added.

dondrzzy

Please refactor

MuhanguziDavid · 2018-11-05T13:19:13Z

Thanks for the feedback, I have refactored accordingly.

- enable user signup from front end [Delivers #160609513]

RonKbS reviewed Nov 2, 2018

View reviewed changes

MuhanguziDavid force-pushed the ft-register-users-160609513 branch 3 times, most recently from dac6dcb to 5828f63 Compare November 3, 2018 20:55

mendozabree reviewed Nov 4, 2018

View reviewed changes

dondrzzy reviewed Nov 5, 2018

View reviewed changes

MuhanguziDavid force-pushed the ft-register-users-160609513 branch from 5828f63 to bfb8e10 Compare November 5, 2018 09:11

MuhanguziDavid force-pushed the ft-register-users-160609513 branch 9 times, most recently from f08746e to cbf40ad Compare November 5, 2018 17:11

feature(users): user registration

5a420ae

- enable user signup from front end [Delivers #160609513]

MuhanguziDavid force-pushed the ft-register-users-160609513 branch from cbf40ad to 5a420ae Compare November 5, 2018 17:14

dondrzzy merged commit 2397d6d into develop Nov 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature(users): user registration #13

feature(users): user registration #13

MuhanguziDavid commented Nov 1, 2018

RonKbS Nov 2, 2018

MuhanguziDavid Nov 3, 2018

RonKbS commented Nov 2, 2018 •

edited

Loading

coveralls commented Nov 3, 2018 •

edited

Loading

mendozabree Nov 4, 2018

MuhanguziDavid Nov 5, 2018

mendozabree left a comment

dondrzzy Nov 5, 2018

dondrzzy Nov 5, 2018

dondrzzy Nov 5, 2018

MuhanguziDavid Nov 5, 2018 •

edited

Loading

dondrzzy Nov 5, 2018

dondrzzy Nov 5, 2018

dondrzzy Nov 5, 2018

dondrzzy Nov 5, 2018

dondrzzy Nov 5, 2018

dondrzzy Nov 5, 2018

dondrzzy Nov 5, 2018

MuhanguziDavid Nov 5, 2018

dondrzzy left a comment

MuhanguziDavid commented Nov 5, 2018 •

edited

Loading

		@@ -0,0 +1,25 @@
		import { REGISTER_USER_SUCCESS, REGISTER_USER_ERROR } from '../actions/types';

feature(users): user registration #13

feature(users): user registration #13

Conversation

MuhanguziDavid commented Nov 1, 2018

What does this PR do?

Description of tasks to be completed?

How should this be manually tested?

What are the relevant pivotal tracker stories?

Any background context you want to add?

Screenshots

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RonKbS commented Nov 2, 2018 • edited Loading

coveralls commented Nov 3, 2018 • edited Loading

Pull Request Test Coverage Report for Build 129

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mendozabree left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MuhanguziDavid Nov 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dondrzzy left a comment

Choose a reason for hiding this comment

MuhanguziDavid commented Nov 5, 2018 • edited Loading

RonKbS commented Nov 2, 2018 •

edited

Loading

coveralls commented Nov 3, 2018 •

edited

Loading

MuhanguziDavid Nov 5, 2018 •

edited

Loading

MuhanguziDavid commented Nov 5, 2018 •

edited

Loading