Admin Actions

[adrian]

Thanks for the report @szabesz - I have now overwritten the PHP time limit for these actions, so that should take care of your timeout issues.

Regarding the duplicate names problem - I have committed a new version which adds logic to check for duplicate usernames and also checks against users already in the system. This all occurs before it adds any users, so it should make for a much nicer experience.

If you have a chance to test, please let me know how it goes.

 

Screenshot showing when duplicates are detected

 

Screenshot showing when existing users are detected

[szabesz]

On 9/19/2017 at 10:06 PM, adrian said:

If you have a chance to test, please let me know how it goes.

I'll get back to this on the weekend and report my findings. Thanks a lot as always!

[szabesz]

@adrian I had some time to test the new version. My findings are:

• usernames with space make the import fail with error (Unknown Selector operator). Previous version worked by trimming space from the beginning and from the end, and at the same time converting spaces to dash found in-between.  I did not notice the latter until now, so last time I got two users with dashes in them. At least now I know I need to fix those manually
• I still got timeout issue with 134 users on MAMP Pro but 32 user were ok to import. Note that current version of MAMP is rather buggy, it might be related to that or it might not. I do not really know.
• Otherwise duplicate user warnings were ok.

For the first time (and only for the very first time) I had an issue reverting back to a database backup after the failed user import because of spaces. I used the core Revert to Backup feature and afterwards the – local staging – site was throwing a lot of:

Warning: Invalid argument supplied for foreach() in .../site/assets/cache/FileCompiler/site/modules/TracyDebugger/TracyDebugger.module on line 1257

I tried clearing the cache directory but it did not help, things got worse, the site was brought down by:

User Error
Exception: You do not have permission to execute this module - ProcessPageView (in .../wire/core/Modules.php line 1236)
#0 .../wire/core/Modules.php(1141): ProcessWire\Modules->getModule('ProcessPageView')
#1 .../index.php(53): ProcessWire\Modules->get('ProcessPageView')

I replaced the database with the one from the production site and all was good afterwards. As I said, it happened only the first time, subsequent reverts in the admin did not produce this.

[adrian]

Thanks @szabesz - I have fixed the username issue in the latest version.

I am not sure what to do about the timeout - I used PHP's set_time_limit to remove the time limit - maybe it's a weird MAMP thing?

Not sure about the backup restore - I am using the core WireDatabaseBackup class to run the restore. Maybe let me know if you come across this again and I'll try to replicate.

Thanks!

[szabesz]

Thanks @adrian!

43 minutes ago, adrian said:

I am using the core WireDatabaseBackup class to run the restore.

I have not yet tried your restore feature, I just made backups with ProcessDatabaseBackups and used those to restore the DB. Anyway, your action helped me a lot as I did not have to write a single line of code and all the users are imported  Thank  you!

[adrian]

Just now, szabesz said:

I have not yet tried your restore feature, I just made backups with ProcessDatabaseBackups and used those to restore the DB.

The automatic backup that occurs when you run an Admin Action is probably best restored with the "Restore" option from the Setup > Admin Actions > Restore menu item. That way you are restoring from just before the action was run giving the best chance of a clean undo.

3 minutes ago, szabesz said:

Anyway, your action helped me a lot as I did not have to write a single line of code and all the users are imported  Thank  you!

You're welcome - thanks for helping to improve it - hopefully now it will be better for others also.

[szabesz]

10 minutes ago, adrian said:

The automatic backup that occurs when you run an Admin Action is probably best restored with the "Restore" option from the Setup > Admin Actions > Restore menu item. That way you are restoring from just before the action was run giving the best chance of a clean undo.

I see, however when the process is aborted with error the only way to revert is to use something else, or am I mistaken? Also, first I tested the import process on my local environment in order to make sure it will probably work in production as well. Also, I always make batch changes to the production site around 5-6AM when actual users are very rare. Our VPS can be automatically restored in seconds provided I made a restore point in advance, of course. I'm always trying to be extra precautious  

Edited September 24, 2017 by szabesz
precautious :)

[Robin S]

Hi @adrian,

Is it possible to include a File field in an action for this module? What I have in mind is uploading a CSV that is then parsed and used as data in the action.

I saw that the Create Users action has a textarea field where you can paste data in CSV format so that is one alternative, but sometimes I think it would be easier to upload an existing CSV file. Ryan's ImportPagesCSV module makes use of a File field so I guess there is a way to use files temporarily in a Process module (I haven't looked into how). If Admin Actions doesn't already support this would you consider adding it?

Also, I noticed a couple of issues while exploring the module. On install I get a PHP notice:

And when I attempt to open the "Ftp Files To Page" action I get an error:

[adrian]

Hi @Robin S - I have fixed both those bugs you reported - thanks for that.

As far as using a File field - actions can use any PW code, so this is certainly possible. You could make use of a FieldtypeFile field using the same approach that Ryan uses in his ImportPagesCSV module, or you could use a Markup field and add a "manual" <input type="file" /> field - whatever you prefer really. I don't think the main code in AdminActions needs any adjustments to support these, but let me know if there is anything I can do.

[Robin S]

protected function defineOptions() {

    return array(
        array(
            'name' => 'csv_upload',
            'label' => 'CSV upload',
            'description' => 'Upload a CSV file.',
            'type' => 'file',
            'extensions' => 'csv txt',
            'overwrite' => true,
            'maxFiles' => 1,
            'required' => true,
        )
    );

}

protected function executeAction($options) {

    // If there was a file uploaded (Pagefiles object)
    if(count($options['csv_upload'])) {
        // Get the file (Pagefile object)
        $file = $options['csv_upload']->first();
        // Parse the CSV data
        $handle = fopen($file->filename, 'r');
        $count = 0;
        while(($row = fgetcsv($handle)) !== false) {
            $count++;
            // Process each $row
            // ...
        }
        fclose($handle);
        // Delete the uploaded CSV file (or else you will get an error next time you upload)
        unlink($file->filename);

        $this->successMessage = "$count CSV rows were processed.";
        return true;
    } else {
        $this->failureMessage = 'Please upload a CSV file';
        return false;
    }

}

 

Edited October 13, 2017 by Robin S
Changed example to use fgetcsv for better handling of new lines within values

[Robin S]

Hi @adrian, a few ideas for tweaks to AdminActions...

• How about adding a $title option inside the action class? To specify a title rather than deriving it from the camel-case in the action class name. It doesn't look quite right if there is a capitalised acronym in the class name.
• Maybe new actions should be automatically enabled for the superuser role? To save having to visit the module config and enable each manually.
• The "Actions" tooltip appears when hovering any part of actions list in the module config (it comes from the title attribute on the inputfield wrapper). This is a bit distracting so maybe remove that?

[adrian]

Hi @Robin S - thanks for those reports/ideas.

• I have added a new $title variable to actions. This is optional, but recommended.
• I have fixed the annoying "Actions" title on hover.

But, regarding having new actions automatically enabled for superusers, I am not so sure. I purposely don't enable any actions for anyone by default. There are a lot there and there will potentially be many more in the future. My thought is that the list could become overwhelming and many/most devs might actually prefer to just enable those that they will use on a semi-regular basis. That said, I am willing to hear more arguments from others in support of this

Cheers!

 

 

[Robin S]

1 hour ago, adrian said:

I purposely don't enable any actions for anyone by default.

I thought all the bundled actions were enabled for superuser by default on install, but now I see that superuser is preselected for all the actions but this doesn't actually take effect until you save the module config.

Maybe that is intentional in Admin Actions, but in general I think that is something it's better to avoid. Users might navigate away without realising they need to save to confirm the settings on screen. And even if they later return to the module config, there is nothing to indicate that the settings they see on screen are not actually live. It can be a problem for the module dev too if you are expecting those default config values that show on screen to actually be available in the module. It's caught me out a few times in my own modules - I've learned now that a better approach is to define the config defaults in the construct() method so that the config that users see is what is actually in effect, right from the moment of install.

Anyway, I see where you're coming from regarding the activation of actions, although personally I don't see any downside to having all the actions active and listed on the Admin Actions page (for superusers) so they can be explored. It's the step of adding actions to the menu where I would be limiting it to just regularly-used actions. 

[szabesz]

4 hours ago, adrian said:

My thought is that the list could become overwhelming and many/most devs might actually prefer to just enable those that they will use on a semi-regular basis.

+1

[adrian]

Just a quick update - I went to use the Email Batcher for the first time with a complex HTML template and realized that ACF and Purifier were getting in the way, so these are now disabled so now you can build complex email templates and preview them right in the CKEditor body field.

[Robin S]

Hi @adrian,

I noticed that in the datatable that lists the actions (at Setup > Admin Actions) the title is not being taken from the $title variable. In the module config datatable it is working.

Config:

Setup > Admin Actions:

[adrian]

Hi @Robin S - it's late here, so need to sleep, but quick questions - is this only a problem for custom "Site" actions? Is it fixed if you save the module config settings page?

Let me know and I'll sort it out in the morning.

[Robin S]

This is a really minor issue so no hurry at all.

2 minutes ago, adrian said:

is this only a problem for custom "Site" actions

No, it affects all actions. I dumped the $info array in the getActionTitle() method and it seems there is no 'title' item present when the actions table is rendered at Setup > Admin Actions.

[adrian]

8 hours ago, Robin S said:

This is a really minor issue so no hurry at all.

No, it affects all actions. I dumped the $info array in the getActionTitle() method and it seems there is no 'title' item present when the actions table is rendered at Setup > Admin Actions.

Something unusual going on here. This is what I see. You can tell I am at Setup > Admin Actions and a bd($info) on line 552 definitely contains a "title". Any chance you could trace it back to find why it's not working at your end please?

[Robin S]

Thanks Adrian, I have sussed it out now.

It's because changes to any of the things that make up the actions info don't get applied until the user revisits the module config page and saves.

What makes it confusing (and this is not a criticism of anything you have done - it's just the way PW treats module config values) is that the updated values are shown in the module config screen, they are just not actually applied. It's the same thing I was whinging about above. I think the PW module config system needs some way to visually indicate which values displayed on a module config screen are saved values and which are not. Because otherwise the expectation is that when you view the module config screen you are viewing actual applied values.

[adrian]

Thanks for the follow up. 

One option is that I could parse the content off all "action.php" files when loading Setup > Admin Actions, but I want to prevent the need to parse all files every time that is loaded, or the Setup > Admin Actions submenu is triggered (assuming you have some actions checked to show in the submenu). That is why that page grabs the $info from the module config settings, rather than the action.php file. I think this is probably still a good idea. 

Correct me if I'm wrong, but the issue you are seeing is only because you created an action, save the config settings page, and then later added a $title but didn't save again after the change? If that's what happened, I don't think we need to worry too much about this - what do you think?

If you feel strongly about that other issue, I am willing to reconsider, or if you feel like submitting a PR with the changes you'd like to see.

Speaking of PRs, am I going to get one with some of your custom actions? If I don't get some soon, I think it's time to ditch all those author info fields - right now that table makes me look like an egomaniac

 

 

[Robin S]

2 hours ago, adrian said:

Speaking of PRs, am I going to get one with some of your custom actions?

I will certainly contribute any actions that would be useful to others, but I only have two so far (the ones you see in my screenshot) and one is a work in progress. The first just an interface for uploading a CSV file, but I don't think this is useful as an action to bundle with the module because the action doesn't do anything by itself - it requires code to process the CSV lines, which would depend entirely on what the user wanted to do.

The second is part of an exploration of how to resize existing images to save on disk space. I have a lot of older sites from before client-side resizing was introduced, and users have uploaded massive images that waste a lot of disk space. I'd like to upgrade PW on these sites and then loop through all the oversized images and replace them with smaller versions. I've worked out how to do this, but again I don't think it makes for a useful action to share. Firstly, for a large list of images this should be broken into smaller jobs that run on a LazyCron (or maybe the new Tasker module) to avoid a timeout. And secondly this is a destructive process that has the potential to do a lot of harm if used incorrectly, so I wouldn't want to put this out there and maybe contribute to a disaster.

 

2 hours ago, adrian said:

Correct me if I'm wrong, but the issue you are seeing is only because you created an action, save the config settings page, and then later added a $title but didn't save again after the change? If that's what happened, I don't think we need to worry too much about this - what do you think?

Yes, that's what happened. No need to change from using the module config data to populate the table - I can understand why it wouldn't be good to parse all the files every time the list is loaded.

But as a more general issue I think that perhaps module developers should rethink how values are populated to the module config page. Currently nearly all modules, mine included, set default/fallback values for config inputfields by just setting the value for the inputfield as it is added to getModuleConfigInputfields() (or one of the many equivalent methods for defining config inputfields). There is probably a tutorial somewhere that started this practice.

The problem with doing this is that when viewing the module config page, there is no way for the user to distinguish between values that are actually saved to the config and values that will be saved to the config if the config form is submitted. I'm sure I'm not alone in expecting that when I view the module config page I am seeing the config values that do exist, not ones that will exist if I submit the form.

Each time I think about this I get different ideas about what the best solution would be. Currently I'm thinking that we module developers should be saving the config values that we want to be the default config as part of the module install method. Then we don't set any default/fallback/other values in getModuleConfigInputfields() besides those coming from the saved module config. That way the user can have confidence that whatever they see in the module config page is data that actually exists in the saved config. And if the module does display config values that are not actually saved yet these should be highlighted in some way. Any thoughts on this?

[adrian]

@Robin S - no worries re your custom actions - I totally understand if they are not ready/relevant for public consumption.

I also understand where you're coming from regarding module default settings. Generally I don't find it an issue because if the settings page hasn't been saved, then the defaults will be used when running the module so effectively what the user sees is what they will get. But, there are exceptions, and this module is certainly one of them. I have a couple of others as well and I do a settings save on module install to solve this - maybe this should be the standard approach? I can't see any downsides, although I wonder if it is safe to actually remove defaults - what if the module was installed, but the settings db data fields was somehow emptied? I guess you'd have to uninstall and reinstall, but it might be a mess of notices in the meantime, but maybe that is ok as this is not a likely scenario.

Not sure how to get everyone on this page though?

 

[Robin S]

1 hour ago, adrian said:

have a couple of others as well and I do a settings save on module install to solve this

It would be good if PW did this by default on module installation. Maybe there is some downside but I can't think of any situations where you want the module config data to look populated (by setting default inputfield values) but not actually be populated.

1 hour ago, adrian said:

I wonder if it is safe to actually remove defaults - what if the module was installed, but the settings db data fields was somehow emptied? I guess you'd have to uninstall and reinstall, but it might be a mess of notices in the meantime

Default values could/should still be set in the construct method so there wouldn't be any notices necessarily. But if the config data was removed from the DB then the config page would appear empty despite default values still being used, which is a different kind of misleading I guess.

I'll ponder it some more.

[szabesz]

10 hours ago, adrian said:

I think it's time to ditch all those author info fields - right now that table makes me look like an egomaniac

Please do not! Don't be like Apple who keeps removing useful features from the system  Jokes aside, this module has not yet received the attention it deserves but things can improve any time. For example as soon as I have the time to implement something useful I will surely contribute.