Skip to content

feat(tables): support custom model resolution logic for inline actions #113

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

Merged
merged 2 commits into from
Jan 24, 2024

Conversation

owenconti
Copy link
Contributor

Similar to the case described here: #112

Same as the other PR, I am happy to update this PR with a fix if you can point me in the direction you want it fixed.


Use case:

We allow users to view their soft deleted records. From this page, they have the ability to restore these records.

Issue

The model look up logic in InvokedActionController can't handle soft deleted records. The issue is on this line:

$model = $table->getModelClass();
$record = $model::findOrFail($data->recordId);

Solving the issue

Ideally there would be a way to define how the action's model is queried for/found. This would allow us to apply a scope during the look up.

Copy link

netlify bot commented Jan 23, 2024

Deploy Preview for hybridly failed.

Name Link
🔨 Latest commit 16d651b
🔍 Latest deploy log https://app.netlify.com/sites/hybridly/deploys/65b14a20d241850008b30bf0

@innocenzi
Copy link
Member

Ideally there would be a way to define how the action's model is queried for/found. This would allow us to apply a scope during the look up.

I like that. My initial reaction was to simply handle the "soft deleted" scenario instead of simply calling findOrFail, but your suggestion is more flexible.

It could be a resolveModelUsing method on the Action instance, which can be called in the InvokedActionController endpoint since the action is already resolved at that point.

Would you mind PRing that?

@owenconti owenconti force-pushed the action-model-lookup branch from c93fffe to bc8ac5c Compare January 24, 2024 16:04
@owenconti
Copy link
Contributor Author

@innocenzi Alright, I've pushed up my first attempt for it. Let me know what you think. Not sure what is going on with the Netlify build, too.

@innocenzi innocenzi changed the title Add failing test for invoking an action on a soft deleted record feat(tables): support custom model resolution logic for inline actions Jan 24, 2024
@innocenzi
Copy link
Member

Thanks, Owen. Looks good to me.

Maybe in the future I'll change this implementation to be more like the bulk action one where you can get the query object without executing it first, but for now this will do 👍

@innocenzi innocenzi merged commit 5e20bc4 into hybridly:0.x Jan 24, 2024
@owenconti owenconti deleted the action-model-lookup branch January 24, 2024 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants