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

[#RESSUP-1455] Added Award lookup field for Institutional Proposal Number #1606

Open
wants to merge 1 commit into
base: contrib
Choose a base branch
from

Conversation

rojlarge
Copy link

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.

.stream()
.map(afp -> afp.getAward().getAwardNumber())
.collect(Collectors.toList());
fieldValues.put("awardNumber", StringUtils.join(awardNumbers, '|'));
Copy link
Contributor

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

@rojlarge
Copy link
Author

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.

@leoherbie
Copy link
Contributor

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);
Copy link
Contributor

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

Copy link
Contributor

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?

Copy link
Author

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.

@rojlarge
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants