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

Add registration UI & routing #27

Merged
merged 5 commits into from Jun 27, 2020
Merged

Conversation

bismitaguha
Copy link
Contributor

Description

  • Add Basic Navbar
  • Add Routing
  • Add Registration component, actions & reducers

Fixes #7

Type of Change:

  • Code
  • Quality Assurance
  • User Interface

Code/Quality Assurance Only

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

How Has This Been Tested?

Server up and running. Cases of possible form validations covered.

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
  • Any dependent changes have been merged

Code/Quality Assurance Only

  • My changes generate no new warnings
  • Any dependent changes have been published in downstream modules

Add Routing
Add Registration component, actions & reducers
<Route {...rest} render={props => (
localStorage.getItem('token')
? <Component {...props} />
: <Redirect to='/register'/>
Copy link
Contributor

Choose a reason for hiding this comment

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

check if the redirect path is correct once.

Copy link

Choose a reason for hiding this comment

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

Can we make use of register() in urls.js instead? @bismitaguha

Copy link

@sidvenu sidvenu left a comment

Choose a reason for hiding this comment

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

Also, please add newlines to the end of files which do not have it.

<Route {...rest} render={props => (
localStorage.getItem('token')
? <Component {...props} />
: <Redirect to='/register'/>
Copy link

Choose a reason for hiding this comment

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

Can we make use of register() in urls.js instead? @bismitaguha

Comment on lines 1 to 52
import axios from 'axios'
import {
urlInfo
} from '../urls'
import {
GET_USER_INFO,
POST_USER_INFO,
USER_INFO_ERRORS
} from './types'

export const getInfo = () => async dispatch => {
try {
const config = {
headers: {
Authorization: `Bearer ${localStorage.token}`,
}
}
const res = await axios.get(urlInfo(), config);
dispatch({
type: GET_USER_INFO,
payload: res.data
});
}
catch (err) {
dispatch({
type: USER_INFO_ERRORS,
payload: err.response.data
})
}
}

export const postInfo = (data) => async dispatch => {
try {
const config = {
headers: {
'content-type': 'application/json',
Authorization: `Bearer ${localStorage.token}`,
}
}
const res = await axios.post(urlInfo(), data, config);
dispatch({
type: POST_USER_INFO,
payload: res.data
});
}
catch (err) {
dispatch({
type: USER_INFO_ERRORS,
payload: err.response.data
});
}
};
Copy link

Choose a reason for hiding this comment

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

Can we rename this file and its methods to a better name? Like wherever you used info simply without context, it could be userinfo?

Comment on lines 1 to 35
import React, { Component } from 'react'
import { connect } from 'react-redux'
import { getInfo } from '../actions/info'
// import { Form, Grid, Image, Divider, Icon } from 'semantic-ui-react'
import PropTypes from 'prop-types'
// import login from './../styles/Login.css'

class Info extends Component {
constructor(props) {
super(props)
}
componentDidMount() {
this.props.getInfo()
}
render() {
return(
<div>
hello
</div>
)
}
}

Info.propTypes = {
userinfo: PropTypes.array.isRequired,
};

const mapStateToProps = state => ({
userinfo: state.info.userinfo,
})

export default connect(
mapStateToProps,
{ getInfo, }
)(Info)
Copy link

Choose a reason for hiding this comment

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

Same as above

Comment on lines 1 to 36
import {
GET_USER_INFO,
POST_USER_INFO,
USER_INFO_ERRORS
} from '../actions/types'

const initialState = {
userinfo: [],
postuserinfo: [],
userinfoerror: null,
}

const infoReducer = (state = initialState, action) => {
switch(action.type) {
case GET_USER_INFO:
return {
...state,
userinfo: action.payload,
};
case POST_USER_INFO:
return {
...state,
postuserinfo: action.payload,
userinfo: [...state.userinfo, action.payload],
}
case USER_INFO_ERRORS:
return {
...state,
userinfoerror: action.payload,
}
default:
return state
}
}

export default infoReducer
Copy link

Choose a reason for hiding this comment

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

Same as above

@abha224
Copy link
Contributor

abha224 commented Jun 17, 2020

The pr was tested locally. Here are the gifs for the same:

  1. To check if the registration is successful.
    register_t1

  2. To check if validation is done properly:

    • every field is mandatory
    • email should be of type xx@xx.xxx
    • passwords should match
    • unique username needed
    • unique email needed

register_t2

register_t3

  1. General comments:
  • the errors messages should be refreshed after every submit so that previous error messages arent visible.
  • password validation not done according to the condition: 1 digit, at least special character, at least uppercase letter, and min length of 12 characters

Copy link

@sidvenu sidvenu left a comment

Choose a reason for hiding this comment

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

@bismitaguha could you make the changes that I suggested in my earlier review? Regarding better names for the dashboard (currently called info/Info).

@bismitaguha
Copy link
Contributor Author

@sidvenu Since the dashboard and navbar issues are different I have covered these in that PR so that there won't be any confusion. This info will just be a basic redirect page that confirms login. I hope that will be cool, else we will be mixing PRs????

@sidvenu
Copy link

sidvenu commented Jun 18, 2020

I get your point. In the perspective of this PR, Info will pertain to a dummy page. Make sure you address the name change when you work on the dashboard UI.

@sidvenu
Copy link

sidvenu commented Jun 18, 2020

I overlooked this in my review earlier, could you add new lines to the end of files you've created in this PR that doesn't have new lines?

@bismitaguha bismitaguha requested a review from sidvenu June 22, 2020 13:29
username: '',
email: '',
password: '',
password2: '',
Copy link
Contributor

Choose a reason for hiding this comment

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

change password2 to confirm_password

import React, { Component } from 'react'
import { connect } from 'react-redux'
import { getInfo } from '../actions/info'
// import { Form, Grid, Image, Divider, Icon } from 'semantic-ui-react'
Copy link
Contributor

@abha224 abha224 Jun 24, 2020

Choose a reason for hiding this comment

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

remove the code commented from this file

src/components/App.js Outdated Show resolved Hide resolved
@abha224
Copy link
Contributor

abha224 commented Jun 24, 2020

@bismitaguha Pls modify the index.html file in public folder appropriately as discussed before.

@bismitaguha
Copy link
Contributor Author

@abha224 Removed comments and made the changes.

src/AuthRoute.js Outdated Show resolved Hide resolved
Copy link
Contributor

@abha224 abha224 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link

@sidvenu sidvenu left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@abha224 abha224 merged commit 8cc2494 into anitab-org:develop Jun 27, 2020
@abha224 abha224 changed the title [WIP]: Add registration UI & routing Add registration UI & routing Jun 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Registration UI
3 participants