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

Teacher merge feature #307

Closed
wants to merge 204 commits into from
Closed

Conversation

ArushC
Copy link
Contributor

@ArushC ArushC commented Apr 15, 2024

What this PR does:

Implements a feature which allows the admin to merge teachers together. I merged the branches of the other pull requests into this one, so do NOT review this pull request until first merging in #295 and #304.

This pull request ALSO includes the email template changes made by @perryzjc and @realmichaeltao . I resolved merge conflicts that came up between our two features. To avoid merge conflicts, @cycomachead it would be best if you reviewed this branch for both the features at once. Here is a summary of the features that are included on this branch:

  1. The teacher merge feature, see description below.

  2. The new email model feature, all relevant PR descriptions are linked here: 187384622/new email model - PR1 - Add new EmailAddress Model without Schema Change on Original Models #308, 187384622/new email model - PR2 - Add Email Address Data Migration Script #309, 187384622/new email model - PR3 - Add schema migration to drop old email columns and Adjust Relevant Dependencies #310, and 187384622/new email model - PR4 - Now add multi-personal-email feature #311

This pull request fixes|implements (pick one...) ______.

TEACHER MERGE FEATURE DESCRIPTION BELOW

Include screenshots, videos, etc.

Screen Shot 2024-04-15 at 11 04 27 AM Screen Shot 2024-04-15 at 11 04 34 AM Screen Shot 2024-04-15 at 11 04 42 AM

Who authored this PR?

@ArushC wrote the tests, implemented the frontend design for the merge page and the frontend/backend for the merge button + modal popup on the teacher show page, and setup routes
@razztech worked on backend for the merge function logic, delivered feedback/guidance on frontend design, modified routes for improved clarity.

How should this PR be tested?

Look at the Cucumber tests to see that everything works.

  • Is there a deploy we can view?

Yes: latest deployment at https://sp24-01-bjc-teachers-d0e972207d82.herokuapp.com/.

  • What do the specs/features test?

They show that stuff works...

  • Are there edge cases to watch out for?

No

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])

razztech and others added 30 commits March 7, 2024 19:41
Country field added for international schools
@razztech razztech force-pushed the teacher-merge-feature branch 3 times, most recently from 70e98f5 to 7fb905a Compare April 19, 2024 23:23
@ArushC ArushC force-pushed the teacher-merge-feature branch from ad38df9 to 7fb905a Compare April 22, 2024 17:24
@ArushC ArushC mentioned this pull request Apr 22, 2024
3 tasks
@razztech razztech force-pushed the teacher-merge-feature branch 2 times, most recently from 332ee31 to ca6c172 Compare April 25, 2024 22:07
@cycomachead cycomachead closed this May 8, 2024
@cycomachead cycomachead deleted the teacher-merge-feature branch May 8, 2024 08:57
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.

5 participants