Allow to configure Originator Field Options

[maennchen]

No description provided.

[izelnakri]

Hi @maennchen could you add more description and better documentation on README.md for this change specifically? I couldnt understand the change completely.

originator_type: Ecto.UUID,
originator_options: [references: :uuid]

why do we need originator_options: [references: :uuid] when originator_type is already Ecto.UUID ?

[maennchen]

@izelnakri This is because my field is not called id but uuid. Therefore the belongs_to references the wrong field in the target table.

[maennchen]

@izelnakri https://hexdocs.pm/ecto/Ecto.Schema.html#module-schema-attributes

@primary_key {:uuid, :binary_id, autogenerate: true}

[maennchen]

@izelnakri I added an example to the README

[izelnakri]

HI @maennchen, thanks for more information and links. As few changes as it may seem, I had to do few investigations, would like to merge this feature after deep and careful thought. So far, here are my remaining concerns:

• We should probally call this option(originator_options) with a more explicit name, since it will allow you to change the options passed into the relationship what do you think of calling it originator_relationship_options?

• I'm bit concerned on how the use-case scenario would play with the rest of what PaperTrail does, or whether it could fall out-of-sync. We probably need a separate test cases where we prove the use-case in code and run Versioning operations on that to verify it passes/serves this use-case well. Thus I would like to hold-off merging this change until we have wide test coverage for this scenario. This could run in our CI as a separate OS process, please have a look at .circleci/config.yml#29.

Thanks again for your suggestions & contributions so far. Looking forward to your tests ;)

[maennchen]

@izelnakri

what do you think of calling it originator_relationship_options?

sure, i'll amend the PR

I'm bit concerned on how the use-case scenario would play with the rest of what PaperTrail does

The relationship itself is completely handled by ecto. If we do not touch private ecto APIs (which we're not), this does not have any direct impact on the library.

I can add a test that uses this behaviour, but I don't think that this needs more than on test that tests if the relationship is correctly resolved.

[maennchen]

@izelnakri Test is added.

[izelnakri]

@maennchen reviewed and run the tests. All good merging and cutting a release now. Thanks!