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

Dataset Publish/Submit for Review - Optimize Button/Popup Code #1664

Closed
mheppler opened this issue Mar 12, 2015 · 5 comments · Fixed by #6909
Closed

Dataset Publish/Submit for Review - Optimize Button/Popup Code #1664

mheppler opened this issue Mar 12, 2015 · 5 comments · Fixed by #6909
Labels

Comments

@mheppler
Copy link
Contributor

mheppler commented Mar 12, 2015

Find a better way to have markup and render logic on the dataset page for all the different Publish/Submit for Review buttons and popups. There are currently eight buttons at the top of the page with seven popups.

  • Submit for Review - inreview.show()
  • Register - registerDataset()
  • Reject - sendBackToContributor.show()
  • Publish - confirmation.show()
  • Publish - releaseDraft.show()
  • Publish - mayNotRelease.show()
  • Publish - publishParent.show()
  • Publish - maynotPublishParent.show()
@mheppler mheppler added the Component: Code Infrastructure formerly "Feature: Code Infrastructure" label Mar 12, 2015
@mheppler mheppler self-assigned this Mar 12, 2015
@mheppler mheppler added this to the Beta 15 - Dataverse 4.0 milestone Mar 12, 2015
@mheppler mheppler modified the milestones: Beta 15 - Dataverse 4.0, In Review - 4.0.x Apr 1, 2015
@mheppler mheppler removed their assignment Jan 19, 2016
@scolapasta scolapasta removed this from the Not Assigned to a Release milestone Jan 28, 2016
@mheppler
Copy link
Contributor Author

mheppler commented May 23, 2017

Looked at this in more detail as part of the work for #3747 and #3849. The main issue is that there are 5 different "Publish" buttons that point to 5 different popups. The render logic for these 5 buttons have two piece of render logic in common:

DatasetPage.dataset.latestVersion.versionState=='DRAFT' 
and permissionsWrapper.canIssuePublishDatasetCommand(DatasetPage.dataset)

Using those as the only render logic, for only one "Publish" button, we can move the rest of the render logic to either the onclick attribute or to the body of the dialog -- which would also allow us to not require 5 different publish popups.

All the conditional render logic needed for the publish popups is determining if we allow you publish or not, and which header, help text and buttons to display in the popup. That logic is not needed in the "Publish" button code and only complicates the button display logic.

This is something worth looking into when we address UI Cleanup - Action Button + Metrics Block Layout #3341.

@mheppler
Copy link
Contributor Author

mheppler commented Feb 1, 2018

Highly related/duplicate of UI Cleanup - Action Button + Metrics Block Layout #3341

@mheppler
Copy link
Contributor Author

mheppler commented Apr 8, 2020

This issue indirectly came up again today when working Layout and styling of action buttons on the dataset and file page #6684.

All these buttons are complicating the render logic required in the new UI which includes a btn-group parent container that must adhere to all these various publish, submit for review, return to author btns. It was also discovered to contain a bug, as it does not work for deaccessioned datasets.

Here is a code snippet removing everything that isn't render logic, which was created to outline how complicated all the render logic can get.

<!-- Publish/Submit for Review/Return to Author Button Group -->
<ui:fragment rendered="#{???}">

<!-- Publish Buttons -->
...
rendered="#{!DatasetPage.dataset.released
              and DatasetPage.dataset.latestVersion.versionState=='DRAFT' 
              and DatasetPage.dataset.owner.released
              and permissionsWrapper.canIssuePublishDatasetCommand(DatasetPage.dataset)}"

...
rendered="#{DatasetPage.dataset.released 
             and DatasetPage.dataset.latestVersion.versionState=='DRAFT' 
             and DatasetPage.dataset.owner.released
             and permissionsWrapper.canIssuePublishDatasetCommand(DatasetPage.dataset)}"

...
rendered="#{DatasetPage.dataset.latestVersion.versionState=='DRAFT' 
              and !DatasetPage.dataset.owner.released
              and permissionsWrapper.canIssuePublishDatasetCommand(DatasetPage.dataset)
              and !DatasetPage.canPublishDataverse()}"

...
rendered="#{DatasetPage.dataset.latestVersion.versionState=='DRAFT' 
             and !DatasetPage.dataset.owner.released
             and DatasetPage.canPublishDataverse()
             and (DatasetPage.dataset.owner.owner == null 
                   or (DatasetPage.dataset.owner.owner != null 
                         and DatasetPage.dataset.owner.owner.released))
                   and permissionsWrapper.canIssuePublishDatasetCommand(DatasetPage.dataset)}"

...
rendered="#{DatasetPage.dataset.latestVersion.versionState=='DRAFT' 
             and !DatasetPage.dataset.owner.released
             and DatasetPage.canPublishDataverse()
             and (DatasetPage.dataset.owner.owner != null 
                    and !DatasetPage.dataset.owner.owner.released)
             and permissionsWrapper.canIssuePublishDatasetCommand(DatasetPage.dataset)}"


<!-- Return to Author Button -->
...
rendered="#{DatasetPage.dataset.latestVersion.versionState=='DRAFT' 
              and DatasetPage.dataset.latestVersion.inReview
              and permissionsWrapper.canIssuePublishDatasetCommand(DatasetPage.dataset)}"


<!-- Submit for Review Button -->
...
rendered="#{DatasetPage.workingVersion == DatasetPage.dataset.latestVersion
                                            and !DatasetPage.datasetLockedInWorkflow
                                            and DatasetPage.dataset.latestVersion.versionState=='DRAFT'
                                            and DatasetPage.canUpdateDataset()
                                            and !permissionsWrapper.canIssuePublishDatasetCommand(DatasetPage.dataset)}"

The render logic that I attempted to use in the UI was from a ui:fragment component around the existing publish/submit/return buttons code (line 156, develop branch).

<!-- PUBLISH DATASET -->
<div class="btn-group btn-group-justified" 
    jsf:rendered="#{DatasetPage.workingVersion == DatasetPage.dataset.latestVersion
        and permissionsWrapper.canIssuePublishDatasetCommand(DatasetPage.dataset)
        or (DatasetPage.dataset.latestVersion.versionState=='DRAFT'
            and DatasetPage.canUpdateDataset()
            and !permissionsWrapper.canIssuePublishDatasetCommand(DatasetPage.dataset))}">

This complicated render logic was not a problem in the old UI, because it only resulted in an empty div that did not impact the UI, so it was never realized that it was faulty. The same render logic used in the new UI layout however results in a Publish btn with an empty dropdown on deaccessioned datasets.

Screen Shot 2020-04-08 at 6 41 16 PM

@mheppler
Copy link
Contributor Author

Thank you, @sekmiller for the commit 6f61923 to the branch for issue #6684. This will hopefully be the first phase of the effort to refactor the code for these buttons outlined in this issue.

@mheppler
Copy link
Contributor Author

mheppler commented Jun 4, 2020

@sekmiller contributed a bunch of render logic optimization code to PR #6909 for issue Layout and styling of action buttons on the dataset and file page #6684. This effort has moved a lot of the render logic of these links/buttons from the XHTML to the DatasetPage.java backing bean. There is now only one publish commandLink in the new button UI xhtml code.

That PR has been linked and will close this issue when it is merged.

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

Successfully merging a pull request may close this issue.

3 participants