In order to make the functionality more reusable, I suggest moving the system display plugin into a submodule on its own, that way it can be used independently of the bundled functionality. It would also resolve problems whereby disabling the module leads to WSOD errors, e.g. as reported in #2386841: Disable all bundled views by default.

CommentFileSizeAuthor
#26 admin_views-n2411193-26.patch11.86 KBdamienmckenna
#18 admin_views-n2411193-18.patch11.49 KBdamienmckenna
#18 admin_views-n2411193-18.interdiff.txt5.1 KBdamienmckenna
#18 admin_views-n2411193-18.zip24.99 KBdamienmckenna
#16 admin_views-n2411193-15.patch8.95 KBdamienmckenna
#16 admin_views-n2411193-15.zip24.95 KBdamienmckenna
#13 admin_views-n2411193-13.interdiff.txt4.4 KBdamienmckenna
#13 admin_views-n2411193-13.patch8.8 KBdamienmckenna
#13 admin_views-n2411193-13.zip24.92 KBdamienmckenna
#11 admin_views-n2411193-11.patch4.84 KBdamienmckenna
#11 admin_views-n2411193-11.interdiff.txt703 bytesdamienmckenna
#11 admin_views-n2411193-11.zip24.62 KBdamienmckenna
#8 admin_views-n2411193-8.zip24.52 KBdamienmckenna
#8 admin_views-n2411193-8.patch4.66 KBdamienmckenna
#5 admin_views-n2411193.zip24.19 KBdamienmckenna
#4 admin_views-n2411193-4.patch4.99 KBdamienmckenna
#3 admin_views-n2411193-3.patch2.72 KBdamienmckenna

Comments

damiankloip’s picture

damiankloip commented

Yes, this is something we have talked about before, some people wanted to use this display plugin only. I am for this.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Title: Move 'system' display time to a submodule » Move 'system' display plugin to a submodule
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Active » Needs review
StatusFileSize
new admin_views-n2411193-3.patch2.72 KB

WIP, needs some testing.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
StatusFileSize
new admin_views-n2411193-4.patch4.99 KB

Some updates - the hook_install() had to be moved over, as did the CSS and the access_menu plugin.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
StatusFileSize
new admin_views-n2411193.zip24.19 KB

FYI I had problems getting the patch to apply, so here's a zip of the new files.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

I suspect it may patch correctly when you use "git apply", I used "patch -p1" and it threw up all over my laptop.

DamienMcKenna queued 4: admin_views-n2411193-4.patch for re-testing.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
StatusFileSize
new admin_views-n2411193-8.patch4.66 KB
new admin_views-n2411193-8.zip24.52 KB

Rerolled.

damiankloip’s picture

damiankloip commented 
  1. +++ b/admin_views.install
@@ -2,15 +2,21 @@
+    drupal_set_message(t('Enabled the new Admin Views System Display submodule.'));

    This should just use $return variable and add this message instead. I don't think a dsm() is useful for things like drush. 

  2. +++ b/admin_views_system_display.install
@@ -0,0 +1,15 @@
+  db_query("UPDATE {system} SET weight = 12 WHERE name = 'admin_views_system_display'");

    Just for completeness, related to this, we should probably reset the admin_view module weight if we only need it here now?

Just a couple of things, otherwise looks fine to me!

chris matthews’s picture

chris matthews commented
Status: Needs review » Needs work
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
StatusFileSize
new admin_views-n2411193-11.zip24.62 KB
new admin_views-n2411193-11.interdiff.txt703 bytes
new admin_views-n2411193-11.patch4.84 KB

So, something like this?

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Needs work

This isn't working right, it totally breaks the admin pages. Dangit.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
StatusFileSize
new admin_views-n2411193-13.zip24.92 KB
new admin_views-n2411193-13.patch8.8 KB
new admin_views-n2411193-13.interdiff.txt4.4 KB

Expanded test coverage to make sure the content admin page loads as intended.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Ok.. there's something weird.. it fails for me sometimes and doesn't other times. Not sure what's going on :-\

Status: Needs review » Needs work

The last submitted patch, 13: admin_views-n2411193-13.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
StatusFileSize
new admin_views-n2411193-15.zip24.95 KB
new admin_views-n2411193-15.patch8.95 KB

Forgot to rename the CSS file.

Status: Needs review » Needs work

The last submitted patch, 16: admin_views-n2411193-15.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
StatusFileSize
new admin_views-n2411193-18.zip24.99 KB
new admin_views-n2411193-18.interdiff.txt5.1 KB
new admin_views-n2411193-18.patch11.49 KB

The test that fails here works locally. I wonder if it's a bug in Views that's fixed in the dev snapshot but is broken in the current 3.22 stable release?

Status: Needs review » Needs work

The last submitted patch, 18: admin_views-n2411193-18.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Related issues: +#2960871: Plan for Views 7.x-3.23 release
damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Let's see if this works now with Views 7.x-3.23..

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
damienmckenna’s picture

damienmckenna
Location TN, USA
commented

And the tests are green! Woot!

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

FYI if you use "patch -p1" to apply the patch it will fail to rename the CSS file, you need to run mv admin_views.css admin_views_system_display.css, otherwise you'll be missing the CSS changes and will get errors saying it can't find the CSS file.

chris matthews’s picture

chris matthews commented
Parent issue: » #3018626: Plan for Administration Views 7.x-1.8 release
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
StatusFileSize
new admin_views-n2411193-26.patch11.86 KB

Rerolled.

  • DamienMcKenna committed 797c115 on 7.x-1.x 
    Issue #2411193 by DamienMcKenna, damiankloip: Move 'system' display...
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs review » Fixed

Committed. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.