Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

OSH: Delete matched facilities from created contributer #2153

Merged
merged 4 commits into from Sep 21, 2022
Merged

OSH: Delete matched facilities from created contributer #2153

merged 4 commits into from Sep 21, 2022

Conversation

mstone121
Copy link
Contributor

@mstone121 mstone121 commented Sep 18, 2022 •
edited

Overview

If a facility has more than one match, those other matches could be promoted when the original facility is deleted. This means that a contributer might need to delete a facility many times to fully delete the facility.

In order to simplify the delete process, facility list items from the same contributor as the original facility should be deleted along with the original facility. This includes the matches associated with these facility list items.

Connects #2124

Notes

This PR has some code with strange looking destructuring with the python double astrisks operator:

                for match in other_matches.filter(**{
                    "facility_list_item__source__contributor":
                    created_by_contributor
                }):

I did this to make the linter happy. I couldn't figure out a way to separate keyword arguments on more than one line (and when it's on one line it's too long for the linter). I'm open to suggestions on better ways to format this.

Testing Instructions

  • ./scripts/manage test api.tests.FacilityDeleteTest

Checklist

  • fixup! commits have been squashed
  • CI passes after rebase
  • CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines
  • This PR is targeted at the correct branch (develop vs. ogr/develop)
  • If this PR applies to both OAR and OGR a companion PR has been created

@jwalgran jwalgran self-requested a review September 19, 2022 15:09
@jwalgran jwalgran self-assigned this Sep 19, 2022
jwalgran
jwalgran approved these changes Sep 19, 2022
View reviewed changes
Copy link
Contributor

@jwalgran jwalgran left a comment

Choose a reason for hiding this comment

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

Excellent work. I tested this manually as well and it works as expected.

src/django/api/tests.py Outdated
response = self.client.delete(self.facility_url)
self.assertEqual(204, response.status_code)

# Both facilities should be deleted because they are
Copy link
Contributor

Choose a reason for hiding this comment

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

Checking the language on this comment.

It says "Both facilities should be deleted.." but based on context I think this should be something like "The facility should be deleted and not be replaced since both items/matches came from the same contributor."

@jwalgran jwalgran assigned mstone121 and unassigned jwalgran Sep 19, 2022
Matt Stone added 2 commits September 19, 2022 17:49
Delete additional matches on facility delete
01d6e77 
If a facility has more than one match, those other matches could be
promoted when the original facility is deleted. This means that a
contributer might need to delete a facility many times to fully delete
the facility.

In order to simplify the delete process, facility list items from the
same contributor as the original facility should be deleted along with
the original facility. This includes the matches associated with these
facility list items.

This commit accomplishes this by adding facility list items from the
same contributor as the created_from contributor of the facility to be
deleted to the list of facility list items to delete.

Additionally, when looking for a best match to promote, facilitiy list
items from the original contributor are excluded. If no best match is
found, nothing changes since all matches and list items are deleted
anyway. If a best match is found, any matches from the original
contributor are deleted along with their list items.
Add tests for same contributor facility deletion
4cd2a18
@mstone121 mstone121 force-pushed the ms/delete-matched-facilities-from-created-contributer branch from e827798 to 4cd2a18 Compare September 19, 2022 22:49
@mstone121 mstone121 changed the base branch from develop to ogr/develop September 19, 2022 22:49
@jwalgran
Copy link
Contributor

@mstone121 Sorry for introducing some confusion when we chatted about develop vs ogr/develop before. We need this specific update to our delete behavior on our production OAR branch (develop) so that we can use it to clean up production data before we transfer it from OAR to OS Hub. The target for this PR can be moved back to develop.

CC @mariel-oar @vrwOAR to double check me on this.

@jwalgran
Copy link
Contributor

OS Hub confirmed that we want this behavior in both branches. We have done this for other PRs by opening a second PR and referencing the first PR in the description.

@mstone121 mstone121 changed the title Delete matched facilities from created contributer OSH: Delete matched facilities from created contributer Sep 20, 2022
@mstone121 mstone121 mentioned this pull request Sep 20, 2022
Merged
@mstone121 mstone121 merged commit a8826f2 into ogr/develop Sep 21, 2022
@mstone121 mstone121 deleted the ms/delete-matched-facilities-from-created-contributer branch September 21, 2022 18:07
@TaiWilkin TaiWilkin added the guest_contributor Issue was completed by someone outside of the CA team label Oct 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jwalgran jwalgran jwalgran approved these changes

Assignees

@mstone121 mstone121

Labels
guest_contributor Issue was completed by someone outside of the CA team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mstone121 @jwalgran @TaiWilkin