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

#164069216 Create descriptive signup/registration validation error messages #11

Merged
merged 1 commit into from
Mar 5, 2019

Conversation

wasibani-roy
Copy link
Contributor

@wasibani-roy wasibani-roy commented Feb 28, 2019

What does this PR do?
This PR creates validation/error messages to give users descriptive feedback on any errors encountered or success archived

Description of the tasks to be completed?

  • Create validation messages to handle email input
  • Create validation messages to handle password input
  • Validate that the email used has not been used before to create an account
  • Validate that the username chosen is not already in use by another person
  • write tests to ensure that the validations are working

How should this be manually tested?

  • clone this repository
  • cd into Ah-backend-aquaman and checkout to branch ft-validate-registration-data-164069216
  • set up your virtual environment
  • Then install the requirements pip install -r requirements
  • Next run following commands
    • pip install -r requirements.txt
    • python manage.py makemmigrations
    • python manage.py migrate
    • python manage.py runserver
  • Navigate to the /api/users/ route and attempt to enter invalid data

What are the relevant pivotal tracker stories?

Screenshots
screenshot 2019-02-28 at 13 01 17

@wasibani-roy wasibani-roy force-pushed the ft-validate-registration-data-164069216 branch from 8a84a43 to e8eadec Compare February 28, 2019 13:54
@CryceTruly
Copy link
Contributor

LGTM

Copy link
Contributor

@CryceTruly CryceTruly left a comment

Choose a reason for hiding this comment

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

LGTM

CryceTruly
CryceTruly previously approved these changes Mar 1, 2019
Copy link
Contributor

@CryceTruly CryceTruly left a comment

Choose a reason for hiding this comment

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

LGTM

BagzieGracious
BagzieGracious previously approved these changes Mar 1, 2019
Copy link
Contributor

@BagzieGracious BagzieGracious left a comment

Choose a reason for hiding this comment

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

LGTM

joelethan
joelethan previously approved these changes Mar 4, 2019
Peace-Apple
Peace-Apple previously approved these changes Mar 4, 2019
Copy link
Contributor

@Peace-Apple Peace-Apple left a comment

Choose a reason for hiding this comment

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

LGTM

@wasibani-roy wasibani-roy force-pushed the ft-validate-registration-data-164069216 branch 6 times, most recently from 4c953eb to 2c89583 Compare March 5, 2019 06:39
Copy link
Contributor

@BagzieGracious BagzieGracious left a comment

Choose a reason for hiding this comment

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

good work, but please can you squash 3 commits to 1

@Peace-Apple
Copy link
Contributor

Good work Roy, however work on squashing the commits

CryceTruly
CryceTruly previously approved these changes Mar 5, 2019
Copy link
Contributor

@CryceTruly CryceTruly left a comment

Choose a reason for hiding this comment

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

LGTM

@wasibani-roy wasibani-roy force-pushed the ft-validate-registration-data-164069216 branch 3 times, most recently from 6b80943 to 42cf531 Compare March 5, 2019 11:15
BagzieGracious
BagzieGracious previously approved these changes Mar 5, 2019
Copy link
Contributor

@BagzieGracious BagzieGracious left a comment

Choose a reason for hiding this comment

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

good work @wasibani-roy, i have looked through it and its really working

@@ -49,3 +42,27 @@ def test_email_exits(self):
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(
response.data['errors']['email'][0], "user with this email already exists.")

def test_register_invalid_email(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the hard, adding other tests and raising the test coverage. but please can you add doc strings for more clarity on your tests.

CryceTruly
CryceTruly previously approved these changes Mar 5, 2019
Copy link
Contributor

@CryceTruly CryceTruly left a comment

Choose a reason for hiding this comment

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

LGTM

joelethan
joelethan previously approved these changes Mar 5, 2019
Peace-Apple
Peace-Apple previously approved these changes Mar 5, 2019
Copy link
Contributor

@Peace-Apple Peace-Apple left a comment

Choose a reason for hiding this comment

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

Good work Roy

- Ensure that email submitted by user is correct
- Ensure that password is more than 8 characters and alphanumeric
- Ensure the the email used has not been used to create a previous account
- Ensure that the username is not already in use within the system
[Finishes #164069216]
Copy link
Contributor

@CryceTruly CryceTruly 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
Contributor

@Peace-Apple Peace-Apple 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
Contributor

@BagzieGracious BagzieGracious left a comment

Choose a reason for hiding this comment

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

@wasibani-roy, i have reviewed your work, you have implemented all the feedback. Thanks for the good work

@marthamareal marthamareal merged commit 05545d9 into develop Mar 5, 2019
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.

None yet

6 participants