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.

Sign up for GitHub

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

Jump to bottom

chore: use a single task to change all permissions #243

Merged
merged 41 commits into from
Nov 2, 2023
Merged

chore: use a single task to change all permissions #243

merged 41 commits into from
Nov 2, 2023

Conversation

shpran
Copy link
Contributor

@shpran shpran commented Oct 5, 2023 •
edited
Loading

@codecov-commenter
Copy link

codecov-commenter commented Oct 5, 2023 •
edited
Loading

Codecov Report

Attention: 147 lines in your changes are missing coverage. Please review.

Comparison is base (d6348db) 44.45% compared to head (5be4913) 39.52%.

Additional details and impacted files 
@@              Coverage Diff              @@
##             develop     #243      +/-   ##
=============================================
- Coverage      44.45%   39.52%   -4.94%     
+ Complexity       233      226       -7     
=============================================
  Files             18       22       +4     
  Lines            857      797      -60     
=============================================
- Hits             381      315      -66     
- Misses           476      482       +6
Files Coverage Δ
model/Command/ChangePermissionsCommand.php 83.33% <100.00%> (-16.67%) ⬇️
model/PermissionProvider.php 10.00% <ø> (ø)
model/SyncPermissionsStrategy.php 84.61% <100.00%> (+1.28%) ⬆️
model/event/DacChangedEvent.php 100.00% <100.00%> (ø)
model/eventHandler/ResourceUpdateHandler.php 100.00% <100.00%> (ø)
model/Command/ChangeAccessCommand.php 97.36% <97.36%> (ø)
model/AdminService.php 0.00% <0.00%> (ø)
model/tasks/ChangePermissionsTask.php 0.00% <0.00%> (ø)
...del/ServiceProvider/PermissionsServiceProvider.php 0.00% <0.00%> (ø)
model/ChangePermissionsService.php 84.05% <84.05%> (ø)
... and 3 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shpran shpran mentioned this pull request Oct 9, 2023
Merged
@hectoras hectoras self-requested a review October 9, 2023 15:39
@shpran shpran requested a review from gabrielfs7 October 9, 2023 18:23
hectoras
hectoras reviewed Oct 10, 2023
View reviewed changes
model/DataBaseAccess.php Outdated Show resolved Hide resolved
@shpran shpran marked this pull request as ready for review October 17, 2023 07:15
@shpran shpran force-pushed the feature/adf-1590/optimize-permissions-task-queue branch from 3c8c831 to 095eb16 Compare October 18, 2023 07:21
@gabrielfs7 gabrielfs7 mentioned this pull request Oct 19, 2023
Merged
@shpran shpran mentioned this pull request Oct 19, 2023
Merged
gabrielfs7
gabrielfs7 reviewed Nov 2, 2023
View reviewed changes
composer.json Outdated
@@ -21,8 +21,8 @@
"minimum-stability": "dev",
"require": {
"oat-sa/oatbox-extension-installer": "~1.1||dev-master",
"oat-sa/generis": ">=15.24",
"oat-sa/tao-core": ">=52.1",
"oat-sa/generis": "dev-feature/adf-1590/optimize-permissions-task-queue as 99.99",
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Update versions

shpran and others added 24 commits November 2, 2023 16:45
@shpran
chore: do not add permissions for the AssessmentContentObject
582af00
@gabrielfs7 @shpran
chore: use methods from ontology to get resources info
35f3c32
@gabrielfs7 @shpran
chore: deal with proper command object rather than unknown array
88ab9ad
@gabrielfs7 @shpran
feat: create new command to change all permissions
b31e61d
@gabrielfs7 @shpran
chore: add possibility to add/revoke access in commands
2eebabc
@shpran
chore: add new event for event log, wrap calls into transactions
27e9e9c
@gabrielfs7 @shpran
chore: use the new method for change access
9484c3c
@gabrielfs7 @shpran
chore: use remove instead of remove
5d82309
@gabrielfs7 @shpran
chore: group information to better delete query + single transaction …
6ba4df9 
…for all operations
@gabrielfs7 @shpran
chore: typo
c9a4e02
@gabrielfs7 @shpran
chore: add unit test for ChangeAccessCommand
56b081d
@gabrielfs7 @shpran
chore: add TODO
8263001
@gabrielfs7 @shpran
refactor!: Removed old methods and PermissionsService which had alrea…
69a1755 
…dy too much deprecations
@gabrielfs7 @shpran
chore: remove TODO for events
dfcdf28
@shpran
chore: fix issue with array unique, add event
58f1c1f
@gabrielfs7 @shpran
chore: remove old methods and add proper signature for new methods
42d6eea
@gabrielfs7 @shpran
chore: add missing typehints and docs
779a565
@gabrielfs7 @shpran
chore: delete in chunks + remove try/catch and delegate to upper level
ec2a06d
@shpran
chore: reduce number of iterations
243d4a6
@shpran
chore: use chunks to save event logs for multiple changes at the same…
3b2828c 
… time
@shpran
chore: add unit test for the new event
2efa8b5
@shpran
chore: check for uniques for userRevokedPermissions variable, change …
888d4f9 
…syntax to PHP 7.4
@shpran
test: add/update unit tests
822ebb3
@shpran
refactor!: remove unnecessary events
f4208c9
@shpran shpran force-pushed the feature/adf-1590/optimize-permissions-task-queue branch from 594964e to f4208c9 Compare November 2, 2023 13:46
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 2, 2023

Version

🚨 Your pull request contains a BREAKING CHANGE, please be sure to communicate it.

Target Version 8.0.0
Last version 7.10.6

There are 2 BREAKING CHANGES, 1 feature, 0 fix

@shpran shpran merged commit 4ccc450 into develop Nov 2, 2023
4 checks passed
@shpran shpran deleted the feature/adf-1590/optimize-permissions-task-queue branch November 2, 2023 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hectoras hectoras hectoras left review comments

@gabrielfs7 gabrielfs7 gabrielfs7 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@shpran @codecov-commenter @hectoras @gabrielfs7