Repository navigation
Validation of script report permissions via study controller actions. - #8252
Conversation
| errors.add(new SimpleValidationError("Unable to find a configured script engine for this report.")); | ||
| return; | ||
| } | ||
| extension = engine.getFactory().getExtensions().getFirst(); |
There was a problem hiding this comment.
ReportUtil just uses extension to find ScriptEngine, we could just pass this in)
There was a problem hiding this comment.
Yes, that seems cleaner
|
|
||
| if (descriptor.isNew()) | ||
| { | ||
| validateScriptCreation(context, report, errors); |
There was a problem hiding this comment.
Why inside isNew() check?
There was a problem hiding this comment.
For existing reports, the permission check happens inside of canEdit
|
Suggest updating study ReportController only allow saving StudyQueryReport and StudyCrosstabReport (and not allow url binding to change report type on edit). |
Good idea, done. FYI, StudyCrosstabReport goes through a different action so it's only StudyQueryReport that uses SaveReport via CreateQueryReport. |
labkey-matthewb
left a comment
labkey-matthewb
left a comment
There was a problem hiding this comment.
Approve, but check with Josh/Adam about target branch.
## Rationale LabKey/internal-issues#1682 Regression test to verify that `study-reports.saveReport.view` handles permission checking properly for script reports. ## Related Pull Requests LabKey/platform#8252
Rationale
https://github.com/LabKey/internal-issues/issues/1682
Validation for
study-reports.saveReport.viewto ensure standard checks are performed for script based reports.Related Pull Requests
LabKey/testAutomation#3273