Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upUP-4065: Portlet Manager -- Detect config/edit/help/about mode support ... #323
Conversation
…t as defined in portlet.xml for portlets published with CPDs
| @@ -41,16 +41,15 @@ | |||
| value="personManager.getPerson(servletRequest)"/> | |||
| <set name="flowScope.portletTypes" | |||
| value="portletAdministrationHelper.getAllowableChannelPublishingDefinitions(person)"/> | |||
| <set name="flowScope.completed" | |||
| value="portlet.id != null and portlet.id != '-1'"/> | |||
| <set name="flowScope.completed" value="!portlet.isNew()"/> | |||
This comment has been minimized.
This comment has been minimized.
apetro
May 15, 2014
Member
| <!-- BASIC INFO view (title, fname, etc.) --> | ||
| <view-state id="basicInfo" model="portlet"> | ||
| <on-entry> | ||
| <evaluate expression="portletAdministrationHelper.prepopulatePortletIfNew(portlet)"/> |
This comment has been minimized.
This comment has been minimized.
apetro
May 15, 2014
Member
Would it be clearer to predicate this call on the new-ness of the portlet, rather than relying on the prepopulatePortlet...() method to condition its behavior on new-ness?
This comment has been minimized.
This comment has been minimized.
drewwills
May 15, 2014
Author
Member
I think it would be -- that was my original desire. I don't know SWF as well as maybe I could, but my research suggested that making this call conditionally would involve adding a <decision-state>, which I felt was a noisy solution.
This comment has been minimized.
This comment has been minimized.
apetro
May 15, 2014
Member
Maybe? Maybe branching the flow depending on whether the portlet is new or existing would be a healthy way to represent the differences between, well, the workflow when new vs existing? diverged flows could merge back together to complete the path for the part where they're the same?
This comment has been minimized.
This comment has been minimized.
drewwills
May 16, 2014
Author
Member
It's already branched new vs. existing... but at at a point where we're not ready to make this choice. We would need to re-branch and re-merge.
|
|
||
| if (!form.isNew()) { | ||
| // Get out; we only prepopulate new portlets | ||
| return; |
This comment has been minimized.
This comment has been minimized.
apetro
May 15, 2014
Member
Should this method have the responsibility of nuancing its behavior depending on new-ness, or should the caller not call this method if the portlet is new?
This comment has been minimized.
This comment has been minimized.
drewwills
May 15, 2014
Author
Member
As above. The (one-and-only) caller is Web Flow, and getting ti to follow nuance is tougher than it sounds.
| * @param form | ||
| */ | ||
| public void prepopulatePortlet(String application, String portlet, PortletDefinitionForm form) { | ||
| public void prepopulatePortletIfNew(PortletDefinitionForm form) { |
This comment has been minimized.
This comment has been minimized.
apetro
May 15, 2014
Member
Thinking about what this method is doing, what its scope and responsibility is, and so what it should be called. Is this more informPortletFormFromPortletType() in that what it's doing is, now that we know the portlet type, binding what is implied by that type into the form representing the publication of an instance of that type?
UP-4065: Portlet Manager -- Detect config/edit/help/about mode support ...
|
@apetro -- Very happy for this flow to continue to evolve, but I think it's time for the existing enhancement to make its way in. |
MUMUP-1711 : Upgrade to weather portlet 1.1.3; add widget config.
drewwills commentedMay 15, 2014
...as defined in portlet.xml for portlets published with CPDs