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

Docs for "Extending the CMS actions" need updating #8773

Open
robbieaverill opened this issue Jan 31, 2019 · 10 comments
Open

Docs for "Extending the CMS actions" need updating #8773

robbieaverill opened this issue Jan 31, 2019 · 10 comments

Comments

@robbieaverill
Copy link
Contributor

Affected Version

4

Description

The documentation for adding/extending CMS actions here don't work in practice:

https://docs.silverstripe.org/en/4/developer_guides/customising_the_admin_interface/how_tos/extend_cms_interface/#extending-the-cms-actions

Assuming someone tries to do this on their DataObject, which is surfaced in the CMS as part of a ModelAdmin, or a GridField via an ORM relationship on a page, $dataObject->getCMSActions() is never called so doesn't show up in the interface.

You can extend GridFieldDetailForm_ItemRequest and updateCMSActions, but you need to do type checking on the record class as well, and this doesn't match the docs.

Another possibility is that we could do this patch, but this logic was the same in SS3 as well as 4, so it still wouldn't have matched the documentation:

diff --git a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php
index 5667ee371..a8facfad0 100644
--- a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php
+++ b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php
@@ -277,7 +277,7 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler
     {
         $canEdit = $this->record->canEdit();
         $canDelete = $this->record->canDelete();
-        $actions = new FieldList();
+        $actions = $this->record->getCMSActions();
         if ($this->record->ID !== 0) {
             if ($canEdit) {
                 $actions->push(FormAction::create('doSave', _t('SilverStripe\\Forms\\GridField\\GridFieldDetailForm.Save', 'Save'))

See https://stackoverflow.com/questions/54251162/silverstripe-4-0-4-getcmsactions-not-working-as-expected-on-dataobject

@jonom
Copy link
Contributor

jonom commented Feb 7, 2020

Seems to be quite a few problems like this in the docs. We should have an international 'fix the docs' working bee 😄 everyone can choose a page, test the examples and make sure they actually work.

@gurucomkz
Copy link
Contributor

This piece of the documentation applies only to the pages.
If you want to do something with any other DataRecord that is displayed via the Grid, check out this - https://forum.silverstripe.org/t/how-to-add-a-new-button-to-cms/2686/4?u=gurucomkz - there's an example on how to actually add something to the footer of the grid-provided editor.

@sheadawson
Copy link

@lekoala to the rescue!

https://github.com/lekoala/silverstripe-cms-actions

@chillu
Copy link
Member

chillu commented Jan 13, 2021

@jonom Keen to organise a working bee? I think that's a great idea. Maybe ask for interest on Slack, and find a way to divy up the work and avoid duplicate effort? Happy to review PRs.

@lekoala
Copy link
Contributor

lekoala commented Jan 14, 2021

@sheadawson thanks for mentioning my module ! I think there are quite a few issues regarding adding actions to dataobjects. You may want to add actions in the modeladmin EditForm, or directly on the row. Look at the amount of code needed for instance in the dhensby/silverstripe-masquerade module, when it should really just be "define the action with it's icon, and run a function that matches the button".

I do believe this kind of stuff deserve to be in the core of the framework. Meanwhile if you find yourself needing to do that kind of stuff, hopefully my module works quite well as far as I can tell :-)

@jonom
Copy link
Contributor

jonom commented Jan 14, 2021

@jonom Keen to organise a working bee?

@chillu I wish Hacktober was closer! Would be a great time to do that as I think participation would be a lot better under that banner. TBH I don't have the capacity to organise this in the short term, but I've put a note in my calendar to revisit this at the start of September... I'm sure the docs will still have some parts that need cleaning up come October 😅 If anyone wants to organise something beforehand though please do and I will surely find some time to review a few pages.

Note to self: git blame could be useful to identify stale sections of docs pages, and significant contributors to certain pages who may be interested in bringing them up to date.

@sminnee
Copy link
Member

sminnee commented Jan 14, 2021

I do believe this kind of stuff deserve to be in the core of the framework. Meanwhile if you find yourself needing to do that kind of stuff, hopefully my module works quite well as far as I can tell :-)

If we could get a PR that improves this without making breaking changes, I'd certainly support it! Probably the general approach would be to add a new API for this purpose and make sure that the current code of say the masquerade module doesn't break.

@lekoala
Copy link
Contributor

lekoala commented Jan 14, 2021

If we could get a PR that improves this without making breaking changes, I'd certainly support it! Probably the general approach would be to add a new API for this purpose and make sure that the current code of say the masquerade module doesn't break.

i'll check if i can do something based on my current codebase and see what can be applied. As explained by @robbieaverill the main starting point in fixing this is to do

- $actions = new FieldList();
+ $actions = $this->record->getCMSActions();

That alone would be a breaking change itself since it would mean that actions defined previously would appear suddenly :-) but i don't think there are a lot of people that would complain that cms actions appear in the cms.

then, the idea is to extend FormAction or create another class like I did. Currently, I'm using a little trick which is to use an array notation (doCustomAction[myAction]) to submit all actions to a single endpoint. The array notation serves as a parameter that allows me to know which action to run on the DataObject. That works well, but it feels like a hack :) (not sure you would like it in the core framework).

Then there are a couple of features related to the save and close, save and next, save and prev. I think the little arrows to navigate on the bottom right are a great addition, but being able to save and navigate is also very useful when batch editing records.

that's the general idea. if we can already do this, that would be a great addition for everyone in my opinion.

@jonom
Copy link
Contributor

jonom commented Jan 14, 2021

Probably the general approach would be to add a new API for this purpose and make sure that the current code of say the masquerade module doesn't break.

Just flagging that if potential changes don't strictly break an API but may nonetheless break another module it may not be a deal-breaker. There's a precedent with the FocusPoint module where a pre-emptive patch was made to support an upcoming core change, so that could be an option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants