Adding support for custom actions per table

[haffi96]

• The general idea to allow custom logic being run for each table, this is more of a POC. So if there is anything wrong with doing this. Let me know.
• I think this is different to hooks. Because hooks run only when a table row is modified 🤔
• First time working with vue and typescript, so would be great to have some suggestions for improvement

For this to work, would need this piccolo-orm/piccolo_api#200. These changes would allow us to send row information to the piccolo_api and run the actions

[lgtm-com]

This pull request introduces 1 alert when merging 4246898 into 2194e43 - view on LGTM.com

new alerts:

• 1 for Wrong name for an argument in a class instantiation

[dantownsend]

@haffi96 This is cool, thanks.

I agree that it's something we need to add.

Some initial thoughts:

• Rather than callbacks, how about we call them 'actions'? I might be biased on the name, as this is what I used in the past with Django.
• It would be good if multiple actions / callbacks could be registered for a single table. So the the API call would be POST /actions/<some-action-name/. Calling GET /actions/ could return the name of all available actions.
• At the moment it seems like the action can only accept a single row as an argument, but it would be great if multiple rows could be passed in.

First time working with vue and typescript, so would be great to have some suggestions for improvement

The Vue and Typescript code looks good.

I think this is different to hooks. Because hooks run only when a table row is modified 🤔

You're right - this is different to hooks, and definitely has it's own unique purpose.

[haffi96]

Rather than callbacks, how about we call them 'actions'?

haha actions was actually my initial choice too (inspired by flask-admin for myself 🤓 ). Decided against it as i saw that there was already some actions related components so didnt want to overlap. But i agree, actions sounds better. Will update

It would be good if multiple actions / callbacks could be registered for a single table

👍🏽

At the moment it seems like the action can only accept a single row as an argument, but it would be great if multiple rows could be passed in.

Yep, that was intentional as i wasn't sure if waiting for the response of an action to process for multiple rows may be kinda weird. But will experiment with this. I agree, would definitely be nice to have it for multiple rows

[lgtm-com]

This pull request introduces 1 alert when merging b3dcd1d into 2194e43 - view on LGTM.com

new alerts:

• 1 for Wrong name for an argument in a class instantiation

[haffi96]

Can register multiple actions per table now and also select multiple rows at once.

Atm, the admin only sends this -> {"table_name": ..... , "row_ids": [...]} back to the user registered custom action functions. We could also send all the row data

And as you can see i've struggled abit with the css 😆 Will have another go at better aligning the drop down buttons

[dantownsend]

@haffi96 Cool, thanks! The UI looks good.

I think passing just the row IDs should be OK.

[dantownsend]

@sinisaos What do you think about this? I think it could be a really useful feature.

[dantownsend]

The tests are just failing because it relies on new features in Piccolo API, which haven't been published to PyPI - we need to prioritise getting them released.

[sinisaos]

@dantownsend I also think it's useful and similar to Django admin actions. Can you also merge this as our main bulk actions so that we have proper bulk update and delete via orm method instead of iterating through a single method like we do now. After that I can change the Piccolo Admin code.

[github-actions]

This PR has been marked as stale because it has been open for 30 days with no activity. Are there any blockers, or should this be closed?

[dantownsend]

Still valid.

[haffi96]

Hey @dantownsend sorry about the delay. I've done some more work on this last week and implemented your suggestions on the piccolo api PR. Just have some unit tests left to do.

But quick question, Is this PR dependent on piccolo-orm/piccolo_api#145 being merged first?

[sinisaos]

But quick question, Is this PR dependent on piccolo-orm/piccolo_api#145 being merged first?

@haffi96 No, this PR is not dependent on piccolo-orm/piccolo_api#145 . It's a completely different PR that provides a solution for propper bulk actions (delete and update) for Piccolo API and Piccolo Admin.

[github-actions]

This PR has been marked as stale because it has been open for 30 days with no activity. Are there any blockers, or should this be closed?