Updated record set ownergroup id as none when zone is private #1321
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1321 +/- ##
=======================================
Coverage 92.05% 92.05%
=======================================
Files 177 177
Lines 7889 7889
Branches 253 227 -26
=======================================
Hits 7262 7262
Misses 627 627 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Using this scenario:
A shared zone is added to vinyl
A user from group A adds a record "foo" to that zone. That record is owned by group A.
The zone is later changed to from shared to private, with the owner group being set to group B.
Testing with these changes, a user from group B will be able to successfully update record "foo". And when you search "foo" in the recordset search, it shows up as being owned by group B. So the functionality in the portal looks good.
However, in the database, the record in the recordset table still shows group A as the owner_group_id, even though it says group B in the portal, which is a bit confusing.
I think as part of these changes we should update the entries in the database as well. Could we have the owner_group_id column update when the zone access type is changed
Nice finding. I noticed it. This is done intentionally because if if we update the DB then when user wants to change zone from private to shared, then the pre - owned records will be updated. It might get confused again right? If we still want to update the DB, we can. |
| @@ -659,6 +660,16 @@ class RecordSetServiceIntegrationSpec | |||
| Some(group2.id) | |||
| } | |||
|
|
|||
| "update successfully for private zone if user is updating record set when record set is already owned by another group were user is not a part of" in { | |||
There was a problem hiding this comment.
A small typo here. Should be where instead of were.
| @@ -184,6 +184,7 @@ class RecordSetService( | |||
| ).toResult | |||
| _ <- if(existing.name == rsForValidations.name) ().toResult else if(allowedZoneList.contains(zone.name)) checkAllowedDots(allowedDotsLimit, rsForValidations, zone).toResult else ().toResult | |||
| _ <- if(allowedZoneList.contains(zone.name)) isNotApexEndsWithDot(rsForValidations, zone).toResult else ().toResult | |||
|
|
|||
There was a problem hiding this comment.
Can remove this new line here.
| @@ -887,6 +887,51 @@ class RecordSetServiceSpec | |||
| result.changeType shouldBe RecordSetChangeType.Update | |||
| result.status shouldBe RecordSetChangeStatus.Pending | |||
| } | |||
|
|
|||
| "return the recordSet change as the result for private zone if user is updating record set " + | |||
| "when record set is already owned by another group were user is not a part of" in { | |||
There was a problem hiding this comment.
Typo here too. were should be replaced by where.
Fixes #1317.
Changes in this pull request: