Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

Add and populate Automatic updates column, add and handle enable/disable auto-updates bulk actions to the multisite themes list table. #33

Merged
merged 5 commits into from Mar 29, 2020

Conversation

pbiron
Copy link
Contributor

@pbiron pbiron commented Mar 25, 2020

Fixes #15.

A few notes:

  1. I put this together by copy-paste-modify the equivalent functions for plugins. Please double check that I correctly replaced all occurrences of plugin with theme (including in comments, etc).
  2. While the hooks exist to add the Auto-update enabled/disabled views to the list table, there is not core hook equivalent to the one we use for the plugins list table to actually restrict the themes shown in the list according to those views. Therefore, this PR does not added those views
    • adding those views when this plugin is merged into core will be no problem
    • we should call this out in the changelog when 0.4.0 is released
  3. I realized when putting this together that we are not currently displaying admin notices when bulk actions are applied for plugins. Therefore, I didn't do them for this PR either. I'll open an issue (and do a separate PR) for that.
  4. I just realized (while typing this) that I forgot to cover the case of the list table in Network > Sites > Edit > Themes. I'll update this PR to cover that shortly.

…ble auto-updates bulk actions to the multisite themes list table.
@pbiron pbiron self-assigned this Mar 25, 2020
@audrasjb audrasjb added the enhancement New feature or request label Mar 25, 2020
@audrasjb audrasjb added this to the 0.4.0 milestone Mar 25, 2020


/**
* Add plugins autoupdates bulk actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Themes, not plugins :)

Copy link
Contributor Author

@pbiron pbiron Mar 25, 2020

Choose a reason for hiding this comment

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

see, I knew I'd probably missed it somewhere :-). Thanks!!

@pbiron
Copy link
Contributor Author

pbiron commented Mar 25, 2020

4. I just realized (while typing this) that I forgot to cover the case of the list table in Network > Sites > Edit > Themes. I'll update this PR to cover that shortly.

That screen should now be covered.

@audrasjb Supporting themes in both of the multisite screens complicated the code a little bit, since both the "action" and "redirect" URLs need to account for both screens. The code could probably use a refactor to make it cleaner, but I think it's good enough to merge and do the refactor separately once the functionality is confirmed to work (there are some other parts unrelated to this PR that I'd like to refactor as well), .

@pbiron pbiron requested a review from audrasjb March 25, 2020 18:17
@pbiron
Copy link
Contributor Author

pbiron commented Mar 25, 2020

3. I realized when putting this together that we are not currently displaying admin notices when bulk actions are applied for plugins. Therefore, I didn't do them for this PR either. I'll open an issue (and do a separate PR) for that.

Turns out that admin notices were correctly showing on non-network screens, and the reason admin notices weren't being show on the network screens is that another hook is needed for that. The recent commit adds that hook...and fixes another copy-past typo.

@audrasjb
Copy link
Contributor

Great job @pbiron thanks!
Looks good to me, let's merge this one now :)

@audrasjb audrasjb merged commit 742eade into master Mar 29, 2020
@audrasjb audrasjb deleted the add/issue-15 branch March 29, 2020 22:59
Copy link
Contributor

@audrasjb audrasjb left a comment

Choose a reason for hiding this comment

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

good to go

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle themes auto-updates in a multisite context
3 participants