BelongsToMany non-pivot action shouldn't use PivotId

[Peterragheb]

• Laravel Version: 10.25.1
• Nova Version: 4.27.13
• PHP Version: 8.1.23
• Database Driver & Version: postgres
• Operating System and Version: macOS 13.5.2
• Browser type and version: Chrome 117.0.5938.132
• Reproduction Repository:

Description:

Selecting resources to run actions on in BelongsToMany field passes the ID field's pivotValue regardless if it's a pivot action or not.

I think this is due to that now in the IndexConcerns.js file since selectedResourceIds returns the pivotValue on the ID field.
The request's toSelectedResourceQuery selects from related table with the pivot ids which returns 0 records and causes the action to return "Sorry! You are not authorized to perform this action."

Detailed steps to reproduce the issue on a fresh Nova installation:

• Create a belongs to many relationship and add action to the related resource (not a pivot action)
• Try running the action

[crynobone]

Unable to reproduce the issue, please provide full reproducing repository based on fresh installation as suggested in the bug report template (or you can refer to https://github.com/nova-issues for example)

[Peterragheb]

Well I found the root cause of the issue. It's because I'm chaining ->withPivot(['id']) on the relationship definition.
In my opinion the pivot id shouldn't take priority over the related model Id when it's not a pivot action. However I'm not sure you consider this a bug and that you'd like to handle this case.

In case you want to reproduce this issue: here's a repo

• run php artisan db:seed (this will create all the needed resources)
• Open the Project resource detail page
• select the user and try to run the "Test Action"

[crynobone]

Pivot ID should have priority when it is available, since we can target the specific IDs instead of just relying on related IDs.

[Grafikart]

Pivot ID should have priority when it is available, since we can target the specific IDs instead of just relying on related IDs.

The behaviour changed between 4.26 and 4.27, without any mention in the changelog so it breaks apps on a minor change (which is unexpected, maybe keep this change for a 5.X version).
In my opinion PivotIDs should be sent in another field and not mixed with "id".

[crynobone]

In my opinion PivotIDs should be sent in another field and not mixed with "id".

This would break other applications as well. Pivot ID should always have priority when it is available.

[Francois-Francois]

This would break other applications as well. Pivot ID should always have priority when it is available.

No because it was not the behavior until this release. What breaks applications now, is this unexpected and undocumented change.

[crynobone]

Sorry but this is the expected behaviour, we probably missing some dusk tests to cover this usage causing the issue to occur.

If someone willing to create a reproducing repository to showcase what need to change I would be willing to take a look.

[Grafikart]

A reproductable repo was already given in previous answer.

If this is the intended behaviour it means Actions are broken on every relation using id as pivot, with a confusing error "you are not authorized..." and no way to fix this issue since the query created inside the action does not handle this correctly (it expects id to be used on the main Model, not the Pivot). The PHP code from toSelectedResourceQuery() does not reflect this intended behaviour. Then a fix could come from an edit to toSelectedResourceQuery()

[Francois-Francois]

@crynobone any feedback ?

[Francois-Francois]

@crynobone just a followup ?

[crynobone]

Still under review. Will update the issue once a PR is in place.

[davidhemphill]

I believe this issue should be fixed. Please post a new issue if the issue persists.

[Peterragheb]

Yes all good now. Thanks!

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.