Community studies#27
Conversation
…ta into community-studies
lose unneeded variable
adjust constant name
fix comment
…ta into community-studies
ryanrdoherty
left a comment
ryanrdoherty
left a comment
There was a problem hiding this comment.
A few easy improvements plus one major bug.
| print(str(datetime.datetime.now()) + " Processing record type: " + recordType['nativeDisplayName'], flush=True) | ||
| searchUrlName = getSearchUrlName(recordType, recordTypeName, paramName) | ||
| runReportToFile(wdkServiceUrl, outputDir, recordTypeName, searchUrlName, paramName, paramValue, recordType, batchType, batchName, batchTimestamp, batchId) | ||
| searchInfo = getSearchInfo(recordType, recordTypeName, paramName) |
There was a problem hiding this comment.
Wondering why you changed the return type of this method to package URL + dynAttrs when you already have them here (using above)?
There was a problem hiding this comment.
not sure i understand.
There was a problem hiding this comment.
Nevermind. Didn't have the whole file open and thought this was part of a different method.
|
|
||
| <wdkModel> | ||
|
|
||
| <constant name="projectIdPropList">projectId</constant> <!-- used by the community studies process query --> |
There was a problem hiding this comment.
Not sure I see the utility of a constant here. It's only used in the XML below right? Might actually be confusing to read the %% macro below and think maybe this is dynamic?
There was a problem hiding this comment.
i did it to highlight that it is a hard-coded value that has to agree with another program
| @Override | ||
| public void validateParameters(PluginRequest request) throws PluginModelException, PluginUserException { } | ||
|
|
||
| static Question getQuestion(PluginRequest request) throws PluginModelException { |
There was a problem hiding this comment.
Remove this and use PluginUtilities.getContextQuestion(request). May need to update EbrcWebSvcCommon since I just added it (already had a RecordClass lookup so very easy add).
There was a problem hiding this comment.
i am not currently depending on ebrc code, so i'm gonna leave this as is
There was a problem hiding this comment.
We could probably move that into WDK... but not today. :)
| WdkModel wdkModel = question.getRecordClass().getWdkModel(); | ||
| List<UserDatasetIds> communityDatasetIds = getCommunityDatasetIds(request, question, wdkModel); | ||
| UserFactory userFactory = new UserFactory(wdkModel); | ||
| List<Long> userIds = communityDatasetIds.stream().map(udi -> udi.ownerId).collect(Collectors.toList()); |
There was a problem hiding this comment.
Probably want to collect into a set here. The method in UserFactory will do it too but maybe be clear that you want to remove duplicates? Just use Collectors.toSet().
ryanrdoherty
left a comment
ryanrdoherty
left a comment
There was a problem hiding this comment.
User IDs in set below (optional since also handled elsewhere). Otherwise approve.
steve-fischer-200
left a comment
steve-fischer-200
left a comment
There was a problem hiding this comment.
all set
3 participants
Not 100% tested, but close enuf for a PR. will complete testing later today, hopefully