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

Fix EZP-26501: In-edit Locations window (legacy) should not force "Visible" flag #24

Merged
merged 1 commit into from
Nov 12, 2016

Conversation

peterkeung
Copy link
Member

JIRA issue ticket: https://jira.ez.no/browse/EZP-26501
PR on eZ repo: ezsystems#1266

Work done to address https://jira.ez.no/browse/EZP-22260 also included this specific commit, whose goal was to save the user's selection of the "hidden flag" toggle in the Locations window in edit mode:

ezsystems@c3e8ff5

However, that commit caused the following:

  • "Visible" is the new default in the Locations window, not "Unchanged"
  • "Visible" overrides any hide effort of a workflow
  • Selecting "Unchanged" as a user and then saving the draft actually saves the selection as "Visible"

Given that:

  • The Locations window in edit mode is only enabled for first drafts. (Ref: http://share.ez.no/learn/ez-publish/node-visibility-hiding-and-revealing-content/(page)/4#eztoc2634_0_4)
  • A node's hidden flag is technically only hidden or not. (Hidden by superior is derived from the parent.)
  • By default a node's hidden flag is "off".
  • The use of the Locations window in edit mode is only for hiding the node; otherwise its hidden flag is technically "unchanged".
  • The effects of the Locations window in edit mode are applied after the content/publish/after trigger.
  • The effects of the Locations window can thus unexpectedly override a content/publish/after workflow if "Visible" is selected.

This is technically true:

  • The "Visible" option is never valid (or at least not a more accurate description than "Unchanged"). Thus it should not be used.

=== How to test this fix ===

First, enable the Locations window: Select "on" for "Locations" under the "User preferences" panel in the right sidebar. Then test the following scenarios:

  • Create a new draft. "Visibility after publishing" should give you the options of "Unchanged" or "Hidden". "Unchanged" should be default.
  • Store the draft with "Unchanged" as the selection. Note that the selection is kept.
  • Store the draft with "Hidden" as the selection. Note that the selection is kept.
  • Edit an existing object. The selection should always show as "Unchanged".

@pkamps
Copy link
Member

pkamps commented Oct 27, 2016

Is there a point allowing the editor explicitly set the visibility to 'visible'?

Also, I do not like the word 'unchanged' because it does not make a lot of sense for new nodes -- they never had a previous value for visibility.

What about allowing the editor to select between:

  • auto
  • visible
  • hidden

The 'auto' value would set the visibility state to 'visible' unless you have custom workflow events (hideuntildate) which calculates and sets the visibility.

@peterkeung
Copy link
Member Author

I believe that "Visible" used to be a valid option because you used to be able to use that Locations window for subsequent drafts too. But eZ removed that option because of conflicts when multiple users were editing an object. Thus, I cannot see a reason for keeping "Visible" because I do not see a case where the default flag on the node itself is not already "Visible".

I think "Unchanged" makes sense because you have the column next to it which is "Current visibility". (See this screenshot: https://dl.dropboxusercontent.com/u/23142/temp/visibility_columns.png.) I'm not wedded to the terminology, though. It's just what's already there.

@pkamps
Copy link
Member

pkamps commented Oct 29, 2016

OK - just having 'unchanged' and 'hidden' are the best option we can have here.

In case of a workflow event which dynamically sets the visibility, we still consider other options like 'Force visible' or 'auto'. We would achieve that by overriding the template and customize the options in the drop-down.

Verdict: +1

@benkmugo
Copy link
Member

As noted by Peter the 'unchanged' & 'hidden' options will do the trick as the 'current visibility' column provides the context necessary.

+1

@pkamps pkamps merged commit 1e3f39a into master Nov 12, 2016
@pkamps pkamps deleted the edit_locations_first_draft branch November 12, 2016 09:27
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