ash-project / ash_paper_trail

The extension for keeping an audit log of changes to your Ash resources.
https://hexdocs.pm/ash_paper_trail
MIT License
28 stars 13 forks source link

belongs_to_actor relationship must be public #47

Closed rgraff closed 3 months ago

rgraff commented 3 months ago

In Ash Paper Trail v0.1.2-rc.0, the belongs_to_actor option will not work if the relationship is not public.

Additional context Ash 3.0.0-rc.21

zachdaniel commented 3 months ago

Ah, so we need to make that attribute_writable? true in that case then.

zachdaniel commented 3 months ago

wait, that already is happening... 🤔

zachdaniel commented 3 months ago

What error does it raise?

rgraff commented 3 months ago

In my use case, the relationship can be nil and it always was. When I make the relationship public, it was being set correctly.

zachdaniel commented 3 months ago

When you get some time could you push a failing test for this? I haven't been able to figure out why this would happen, and haven't had time to reproduce either.

zachdaniel commented 3 months ago

When you get some time could you push a failing test for this? I haven't been able to figure out why this would happen, and haven't had time to reproduce either.

nallwhy commented 3 months ago

@zachdaniel I'm handling this issue.

nallwhy commented 3 months ago

@zachdaniel This commit reproduces the issue. https://github.com/nallwhy/ash_paper_trail/commit/2ff9ff9dfef902c5c1bf6472cb9d22fb369f45b6

The issue occurs because the create action created in AshPaperTrail.Resource.Transformers.CreateVersionResource uses default_accept :*, which does not include attributes with public?: false.

nallwhy commented 3 months ago

I think this is related to https://github.com/ash-project/ash/pull/1053. If default_accept :* is a backwards compatibility tool, ash extensions shouldn't use that. It's not all attributes, but public all attributes.

nallwhy commented 3 months ago

I think It can be fixed with a transformer that creates actions accepting all attributes. What do you think?

zachdaniel commented 3 months ago

You're correct about the problem, good call. I would say we can do this w/o a transformer. get all attributes in the transformer that we're in, and then add accept unquote(all_attributes) to the actions we create.