-
Notifications
You must be signed in to change notification settings - Fork 8
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
187384622/new email model - PR4 - Now add multi-personal-email feature #311
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
187129573 change teacher status
187186207/iter1 golden repo fixes
Country field added for international schools
187384622/new email model - PR1 - Change Email and Make Sure original tests didn't break
…187384622/new-email-model-pr2
187384622/new email model - PR2 - Add Email Address Data Migration Script
187375265/fix teachers controller
187384622/new email model - PR3 - Add schema migration to drop old email columns and Adjust Relevant Dependencies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Original PR
Pivotal Tracker Link
What this PR does:
This pull request implements the multi-personal-email feature, with main changes on the view, a few changes on the backend to simplify the email processing logic, no harm on db.
Include screenshots, videos, etc.
Updates on Create Teacher Form
Based on Michael's advice, the code would be easier to manage by removing personal email field from the original form, and make it as an individual form page.
Now, even as an admin, you can't modify the personal email field in the regular form.
View
At the view Teacher page, now there is a new button to edit teacher's personal emails. And u can see the personal emails displayed at this view page, in multi lines.
Create & Updates & Delete
Error Vadalitions
Repeated Emails in the DB
Frontend Validation on Empty Values & Invalid Format
Backend Validation on Invalid Format
This needs to be done by Rspec. It's essentially validated by EmailAddress Model itself, and it's covered by the test on
email_addresses_controller_spec.rb
Who authored this PR?
How should this PR be tested?
Test the Basic CRUD operations + error validation
Are there any complications to deploying this?
Nope
Checklist:
michael/12345-add-new-feature
Any branch name will do as long as the story ID is there. You can usegit checkout -b [new-branch-name]
)