-
Notifications
You must be signed in to change notification settings - Fork 61
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
[#RESSUP-1455] Added Award lookup field for Institutional Proposal Number #1606
base: contrib
Are you sure you want to change the base?
Conversation
.stream() | ||
.map(afp -> afp.getAward().getAwardNumber()) | ||
.collect(Collectors.toList()); | ||
fieldValues.put("awardNumber", StringUtils.join(awardNumbers, '|')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably use stream.reduce rather than stringutils.join
Thanks for pointing that out Travis, that's definitely better than StringUtils.join. However, it looks like collecting with Collectors.joining is the cleanest option. |
I agree. The concept is basically the same as a reduce in this case :) |
if (StringUtils.isBlank(awardNumbers)) { | ||
return new ArrayList<>(); | ||
} | ||
fieldValues.put("awardNumber", awardNumbers); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If awardNumber is set as a search criteria then append. Don't just overwrite the value with the map.put
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related, @rojlarge did you try just renaming the field to fundingProposals.proposal.proposalNumber and not doing this search separately? That way hopefully OJB would do all the necessary joins to make it happen automatically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I was just looking at that... I swear there was a reason initially that we couldn't do that, but I just tried removing the changes to AwardLookupableHelperServiceImpl.java entirely (the lookup field is already named "fundingProposals.proposal.proposalNumber" in Award.xml) and it seems to work just fine. I can take another look at this tomorrow to make sure I'm not missing anything else, but doing it that way is way cleaner and handles the "awardNumber" case w/ wildcarding correctly.
I did some more testing, and just using the correct field name works fine. I've updated this branch to only include the Award.xml change, and rebased it off of the latest master as well. |
RESSUP-1455
This pull request adds an "Institutional Proposal ID" field to the Award lookup, which returns the active version of Awards associated with any version of the requested IP.