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

187384622/new email model - PR4 - Now add multi-personal-email feature #54

Merged
merged 7 commits into from
Apr 19, 2024

Conversation

perryzjc
Copy link
Member

@perryzjc perryzjc commented Apr 19, 2024

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.
image

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.
image

Create & Updates & Delete

image

Error Vadalitions

Repeated Emails in the DB
image
Frontend Validation on Empty Values & Invalid Format
image image
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?

  1. Perry (Jingchao) Zhong @perryzjc
  2. Michael Tao (@realmichaeltao )

How should this PR be tested?

  • Is there a deploy we can view?
  • What do the specs/features test?
    Test the Basic CRUD operations + error validation
  • Are there edge cases to watch out for?

Are there any complications to deploying this?

Nope

Checklist:

  • Has this been deployed to a staging environment or reviewed by a customer?
  • Tag someone for code review (either a coach / team member)
  • I have renamed the branch to match PivotTracker's suggested one (necessary for BlueJay) (e.g. michael/12345-add-new-feature Any branch name will do as long as the story ID is there. You can use git checkout -b [new-branch-name])

@perryzjc perryzjc marked this pull request as ready for review April 19, 2024 02:41
@perryzjc
Copy link
Member Author

There was a merge conflict and I resolved it in this commit [Merge branch 'main' into 187384622/new-email-model-pr4](5e098f4).

Again, GitHub have this weird feature which merges main back to current branch to resolve the merge conflict.

@ArushC
Copy link

ArushC commented Apr 19, 2024

This looks great!

@ArushC ArushC merged commit 096f456 into main Apr 19, 2024
6 checks passed
@ArushC ArushC deleted the 187384622/new-email-model-pr4 branch April 19, 2024 17:16
@perryzjc perryzjc restored the 187384622/new-email-model-pr4 branch April 19, 2024 19:04
@cycomachead cycomachead deleted the 187384622/new-email-model-pr4 branch May 8, 2024 08:58
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.

2 participants