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

UP-4964: Fix configuration of BranchingRenderingPipeline on uP5 #1050

Merged
merged 8 commits into from
Nov 22, 2017

Conversation

drewwills
Copy link
Contributor

@drewwills drewwills commented Nov 21, 2017

https://issues.jasig.org/browse/UP-4964

Checklist
Description of change

The configuration patterns we adopted for uP5 are a big improvement. Nevertheless, some uPortal capabilities are not yet manageable in the uP5 way. One such capability is uPortal Rendering Pipeline customization based on branching. This PR aims to address this bug -- to make it possible to configure branching pipeline settings properly in uPortal-start.

I intend to add documentation for these features before removing the [WIP] tag.

…ipelineTerminatorTest.java, FocusedOnOnePortletPredicateTest.java, and ProfileFNamePredicateTest.java into the uPortal-rendering module (where they belong)
… java.util.function.Predicate (Java 8 native)
…chPoint to make it easier to configure these (existing) capabilities in the uP5 way
@ChristianMurphy
Copy link
Member

AppVeyor is failing due to unrelated issue from https://issues.jasig.org/browse/UP-4965

@ChristianMurphy ChristianMurphy added this to the 5.0.1 milestone Nov 21, 2017
private IPortalRenderingPipeline alternatePipe;

public void setOrder(int order) {
this.order = order;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ what happens if two branch points have the same order?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of them (randomly selected) will be processed first. IMHO that's fine. The alternative would be detecting the circumstance and throwing an exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps logging a warning is a good middle ground.

…ain SemVer; check for multiple RenderingPipelineBranchPoint with the same 'order' value and throw an exception in case of collision
@drewwills drewwills changed the title [WIP] UP-4964: Fix configuration of BranchingRenderingPipeline on uP5 UP-4964: Fix configuration of BranchingRenderingPipeline on uP5 Nov 22, 2017
@drewwills
Copy link
Contributor Author

I removed the [WIP] label because this change is ready to merge, afaik.

@drewwills drewwills merged commit 57b15da into uPortal-Project:master Nov 22, 2017
@drewwills drewwills deleted the UP-4964 branch November 22, 2017 18:41
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.

None yet

2 participants