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

[Batch] Change State: pass only affected items to table and event #22851

Merged

Conversation

@dneukirchen
Copy link
Contributor

dneukirchen commented Oct 29, 2018

Summary of Changes

When you currently change the state of items (published, unpublished, trashed...)in the list via inline or toolbar button, joomla does not care about the current state of the item.

This affects the notification (x items published...) and more importantly it also affects the primary keys that are passed to the onContentChangeState event (it passes all primary keys of all selected items including the ones where the state did not change). A plugin that listens to this event has no possibility to check if i.e. an article was unpublished before, because the value in db already changed.

After these changes we pass only the pks that really should be/changed to the table and plugin event. If there are no items to change, we respond with a success message "0 items published".

Because we already load the item, no additional query is needed.

Testing Instructions

Go to article Manager, set one article as unpublished.
Checkall articles and publish them.

bildschirmfoto 2018-10-29 um 14 12 54

As the change was made in ArticleModel this will affect all Models in the backend, that are extending it. So it would be good to test other entities as well.

Before Changes:

bildschirmfoto 2018-10-29 um 14 13 13

After Changes:

bildschirmfoto 2018-10-29 um 14 13 35

When no items were published:

bildschirmfoto 2018-10-29 um 14 14 47

dneukirchen added 2 commits Oct 29, 2018
@bembelimen

This comment has been minimized.

Copy link
Contributor

bembelimen commented Oct 29, 2018

I have tested this item successfully on 3544a47


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22851.

1 similar comment
@Quy

This comment has been minimized.

Copy link
Contributor

Quy commented Oct 29, 2018

I have tested this item successfully on 3544a47


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22851.

@Quy

This comment has been minimized.

Copy link
Contributor

Quy commented Oct 29, 2018

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/22851.

@joomla-cms-bot joomla-cms-bot added the RTC label Oct 29, 2018
@mbabker mbabker added this to the Joomla 3.9.1 milestone Nov 5, 2018
@mbabker mbabker merged commit efd10de into joomla:staging Nov 5, 2018
5 checks passed
5 checks passed
Hound No violations found. Woof!
JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone/pr the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@joomla-cms-bot joomla-cms-bot removed the RTC label Nov 5, 2018
@dneukirchen dneukirchen deleted the dneukirchen:feature/publish-only-affected-pks branch Nov 6, 2018
@Quy

This comment has been minimized.

Copy link
Contributor

Quy commented Nov 6, 2018

@dneukirchen Under Extensions > Plugins, it is not possible to toggle under the Status column.

@dneukirchen

This comment has been minimized.

Copy link
Contributor Author

dneukirchen commented Nov 6, 2018

Will take a look tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.