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

[Acl] Fix for issue #9433 #9485

Closed
wants to merge 2 commits into
base: 2.2
from

Conversation

Projects
None yet
2 participants
@guilro
Contributor

guilro commented Nov 10, 2013

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #9433
License MIT
Doc PR

Two new test for issue #9433 :
testUpdateAclInsertingMultipleObjectFieldAcesThrowsDBConstraintViolations()
testUpdateAclDeletingObjectFieldAcesThrowsDBConstraintViolations()

The change to updateAces() line 857 is enough to make the first test succeed. When changing the order field value to a higher value, we must first change the value of the next entry (and all the next entries recursively) to preserve uniqueness of the order field in the database.

All the other changes are for the second test. In the former updateAcl() method, we commit the changes of the existing ACEs to the database before deleting or adding the new ones. We must delete the old ACEs before changing the existing ACEs in order to preserve uniqueness of the order field in the database.

fabpot added a commit that referenced this pull request Nov 22, 2013

bug #9485 [Acl] Fix for issue #9433 (guilro)
This PR was squashed before being merged into the 2.2 branch (closes #9485).

Discussion
----------

[Acl] Fix for issue #9433

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #9433
| License       | MIT
| Doc PR        |

Two new test for issue #9433 :
`testUpdateAclInsertingMultipleObjectFieldAcesThrowsDBConstraintViolations()`
`testUpdateAclDeletingObjectFieldAcesThrowsDBConstraintViolations()`

The change to `updateAces()` line 857 is enough to make the first test succeed. When changing the `order` field value to a higher value, we must first change the value of the next entry (and all the next entries recursively) to preserve uniqueness of the `order` field in the database.

All the other changes are for the second test. In the former `updateAcl()` method, we commit the changes of the existing ACEs to the database before deleting or adding the new ones. We must delete the old ACEs before changing the existing ACEs in order to preserve uniqueness of the `order` field in the database.

Commits
-------

a38fab9 [Acl] Fix for issue #9433

@fabpot fabpot closed this Nov 22, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment