Deprecate study protocol designer tools (phase 2)#6406
Conversation
…he module classes
| private void importStudyDesignData(BindException errors, VirtualFile studyDir, StudyImportContext importContext) throws Exception | ||
| { | ||
| if (importContext != null) | ||
| if (importContext != null && OptionalFeatureService.get().isFeatureEnabled(StudyUtils.STUDY_DESIGN_FEATURE_FLAG)) |
There was a problem hiding this comment.
I believe that the importTreatmentVisitMapData method below should have this same check
| @@ -2783,10 +2784,6 @@ public void deleteAllStudyData(Container c, User user) | |||
|
|
|||
| deleteStudyDesignData(c, user, studyDesignTables); | |||
There was a problem hiding this comment.
should this deleteStudyDesignData be moved as well?
There was a problem hiding this comment.
I'll probably save that for the next phase when we split it out to a separate module
| if (null != contextualRole && ReaderRole.class != contextualRole.getClass()) | ||
| throw new IllegalStateException("Only ReaderRole is currently supported"); | ||
| _contextualRole = contextualRole; | ||
| _designQuerySchema = StudyDesignQuerySchema.get(this, c, study, contextualRole); |
There was a problem hiding this comment.
the check for isFeatureEnabled happens in StudyDesignQuerySchema right? so it isn't needed here
There was a problem hiding this comment.
Yes, it will return null in that case and there should be null checks throughout this class.
| { | ||
| checkedPermissions.add(perm); | ||
| // Most tables should not be editable in Dataspace | ||
| if (!perm.equals(ReadPermission.class) && getContainer().isDataspace()) |
There was a problem hiding this comment.
if Atlas is the only server using study design, we probably don't really need these Dataspace related checks anymore right? probably not something that needs to change in this PR though
There was a problem hiding this comment.
Yeah, that's a good question, I found these Dataspace checks confusing so it will be nice to clean these up in the final phase.
| BaseColumnInfo containerCol = new AliasedColumn(this, "Container", _rootTable.getColumn("Container")); | ||
| containerCol.setConceptURI(BuiltInColumnTypes.CONTAINERID_CONCEPT_URI); | ||
|
|
||
| addColumn(containerCol); |
There was a problem hiding this comment.
it looks like StudyDesignBaseTable. addContainerColumn matches these lines, can that be used here?
| protected boolean checkContainerPermission(UserPrincipal user, Class<? extends Permission> perm) | ||
| { | ||
| return _userSchema.getContainer().hasPermission(user, perm, getContextualRoles()); | ||
| }*/ |
There was a problem hiding this comment.
can these commented out methods be removed? I think these are in the parent StudyDesignBaseTable right?
cnathe
left a comment
cnathe
left a comment
There was a problem hiding this comment.
I pulled down the PR and did some minor manual testing to verify the study design tables show in the study schema when the flag is enabled (and do not show when disabled).
2 participants
Rationale
This is the follow on work for this PR. For this phase of the work, the tables used by the study protocol designer will not be shown in the
studyschema. Additionally export and import options for this feature will not be available.Related Pull Requests
Changes
StudyDesignQuerySchemaandStudyDesignSchemato manage the study design tables.