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

Revive PR#1067 #1139

Merged
merged 2 commits into from
Oct 11, 2023
Merged

Revive PR#1067 #1139

merged 2 commits into from
Oct 11, 2023

Conversation

jim
Copy link
Member

@jim jim commented Oct 3, 2023

This PR will merge in the work done by @stephanieminn in #1067:

What it does

  • Allows members to remove holds that are associated with an appointment, and ensures that the hold removed from the appointment.
  • If the hold was the only item in the appointment, the appointment is canceled.

Why it is important

Resolves #802 and #666!

UI Change Screenshot

Alert when a user tries to remove a hold associated with an appointment:

image

Implementation notes

I noticed that Hold was defined to have a has_many relationship with appointments, but experimenting with the app led me to believe that it shouldn't be possible for a hold to be added to any other appointment. As a result, I did a lil refactor to enforce that a hold has_one appointment relationship to make the subsequent work easier.

I also added a dependent: :destroy strategy for appointment holds to avoid orphaned records when a hold is destroyed. If these were the wrong assumptions, please let me know!

In reality, a hold can only be have one scheduled appointment at a time.
Also makes sure that when a hold is destroyed, its associated
appointment hold join record is destroyed.
If a hold is canceled by a member, it is removed from any associated
appointments. If it was the only item in the appointment, the
appointment is also canceled.
@jim jim force-pushed the sm-remove-hold-from-appointment branch from 7163b0b to 58f41fc Compare October 3, 2023 02:24
@jim jim changed the title Pull in PR 1067 Revive PR#1067 Oct 3, 2023
Copy link
Contributor

@phinze phinze left a comment

Choose a reason for hiding this comment

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

This looks pretty straightforward!

@jim jim merged commit c3c1f99 into main Oct 11, 2023
5 checks passed
@jim jim deleted the sm-remove-hold-from-appointment branch October 11, 2023 22:08
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.

Removing a hold will also remove it from an appointment
3 participants