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

[data_release] Introduce Manage File, Revamp Manage Permissions, Hide, Delete #9445

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

skarya22
Copy link
Contributor

@skarya22 skarya22 commented Nov 4, 2024

Brief summary of changes

  • Introduces deleting, and hiding files. A user needs the data_release_hide permission to be able to hide a file.
  • Introduces a new manage file form accessible by the 'pencil' beside a file name. This lets the user hide or delete the file
  • Revamps the manage permissions form to load faster and be easier to use by having dropdowns instead of a long, slow list

Testing instructions (if applicable)

  1. Try adding and removing permissions from the new 'Manage File Form' which is accessible by the 'pencil' icon beside a file name
  2. Try hiding a file in the same menu, and then deleting a file. A hidden file will be highlighted in red on the data release page, and is only visible to the user who hid it, and admin users. A deleted file is removed from the data_release table.
  3. Try changing version permission access from the Manage permission form. Attempt to add permissions to a specific version for a user, then try removing the user from the version

CCNA OVERRIDE PR
CCNA OVERRIDE PR

@kongtiaowang kongtiaowang added the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Nov 13, 2024
}

$DB = $this->loris->getDatabaseConnection();

Copy link
Contributor

@kongtiaowang kongtiaowang Nov 13, 2024

Choose a reason for hiding this comment

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

missing " $DB->delete('data_release_permissions',[ 'data_release_id' => $data_release_id,]);" here. Otherwise it will cause 500 error when delete a file.
[Integrity constraint violation: 1451 Cannot delete or update a parent row: a foreign key constraint fails (wangshen_dev_hbcd.data_release_permissions, CONSTRAINT FK_data_release_id FOREIGN KEY (data_release_id) REFERENCES data_release (id))#0 /var/www/Loris/php/libraries/Database.class.inc(624): PDO->exec()\n#1 /var/www/Loris/modules/data_release/php/files.class.inc(402): Database->delete()\n#2 /var/www/Loris/modules/data_release/php/files.class.inc(68): ]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you!

@skarya22 skarya22 removed the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Nov 13, 2024
@kongtiaowang
Copy link
Contributor

kongtiaowang commented Nov 14, 2024

I hided a file using admin account, Then I created a new account with all data_release permissions. the data_release page can't load.
" Column not found: 1054 Unknown column 'u.ID' in 'where clause'#0 /var/www/Loris/src/Data/Provisioners/DBRowProvisioner.php(89): PDOStatement->execute()\n#1 /var/www/Loris/src/Data/ProvisionerInstance.php(117): LORIS\Data\Provisioners\DBRowProvisioner->getAllInstances()\n#2 /var/www/Loris/src/Data/Table.php(68): LORIS\Data\ProvisionerInstance->execute()\n#3 /var/www/Loris/src/Data/Table.php(81): LORIS\Data\Table->getRows()\n#4 /var/www/Loris/src/Data/Table.php(99): LORIS\Data\Table->toArray()\n#5 /var/www/Loris/php/libraries/DataFrameworkMenu.class.inc(149): LORIS\Data\Table->toJSON()\n#6 /var/www/Loris/php/libraries/NDB_Menu_Filter.class.inc(645): DataFrameworkMenu->toJSON()\n#7 /var/www/Loris/php/libraries/NDB_Page.class.inc(744): NDB_Menu_Filter->display()\n#8 /var/www/Loris/php/libraries/DataFrameworkMenu.class.inc(204): NDB_Page->handle()\n#9 /var/www/Loris/php/libraries/NDB_Page.class.inc(721): DataFrameworkMenu->handle()\n#10 /var/www/Loris/php/libraries/DataFrameworkMenu.class.inc(186): NDB_Page->process()\n#11 /var/www/Loris/php/libraries/Module.class.inc(322): DataFrameworkMenu->process()\n#12 /var/www/Loris/src/Middleware/ResponseGenerator.php(51): Module->handle()\n#13 /var/www/Loris/src/Middleware/AuthMiddleware.php(64): LORIS\Middleware\ResponseGenerator->process()\n#14 /var/www/Loris/src/Router/ModuleRouter.php(75): LORIS\Middleware\AuthMiddleware->process()\n#15 /var/www/Loris/src/Middleware/ExceptionHandlingMiddleware.php(55): LORIS\Router\ModuleRouter->handle()\n#16 /var/www/Loris/src/Router/BaseRouter.php(138): LORIS\Middleware\ExceptionHandlingMiddleware->process()\n#17 /var/www/Loris/src/Middleware/ResponseGenerator.php(51): LORIS\Router\BaseRouter->handle()\n#18 /var/www/Loris/src/Middleware/ContentLength.php(53): LORIS\Middleware\ResponseGenerator->process()\n#19 /var/www/Loris/htdocs/index.php(74): LORIS\Middleware\ContentLength->process()\n#20 {main},

@skarya22
Copy link
Contributor Author

@kongtiaowang Fixed! Thank you so much for catching that. Oversight on my part

@kongtiaowang kongtiaowang added the Passed manual tests PR has been successfully tested by at least one peer label Nov 19, 2024
Copy link
Contributor

@kongtiaowang kongtiaowang left a comment

Choose a reason for hiding this comment

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

LGTM

@maximemulder maximemulder added Project: CCNA Issue or PR related to the CCNA project and removed Priority: Projects labels Nov 29, 2024
@skarya22 skarya22 self-assigned this Jan 23, 2025
@skarya22 skarya22 added the State: Needs work PR awaiting additional work by the author to proceed label Jan 23, 2025
@skarya22 skarya22 removed the State: Needs work PR awaiting additional work by the author to proceed label Jan 28, 2025
@skarya22 skarya22 requested a review from driusan January 28, 2025 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Passed manual tests PR has been successfully tested by at least one peer Project: CCNA Issue or PR related to the CCNA project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants