Skip to content
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

Ability to delete multiple features and to initialise existing record #97

Closed
wants to merge 7 commits into from

Conversation

marioimr
Copy link
Contributor

@marioimr marioimr commented Jul 8, 2020

  • change to PaperTrail.Multi.delete/3 to enable reception of model_key and version_key to execute multiple deletions (on the model of Ecto.Multi.insert/3
  • add PaperTrail.initialise/2 to have function that explicitly initialise an already existing record into versions table

@@ -12,6 +12,22 @@ defmodule PaperTrail do
defdelegate get_versions(model, id, options), to: PaperTrail.VersionQueries
defdelegate get_current_model(version), to: PaperTrail.VersionQueries

@doc """
Intialise paper_trail for existing record
Copy link
Owner

@izelnakri izelnakri Jul 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about this? "Explicitly inserts an already non-versioned existing record into versions table"

@@ -110,11 +110,14 @@ defmodule PaperTrail.Multi do
def delete(
%Ecto.Multi{} = multi,
struct,
options \\ [origin: nil, meta: nil, originator: nil, prefix: nil]
options \\ [origin: nil, meta: nil, originator: nil, prefix: nil, model_key: :model, version_key: :version]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call, thanks!

@izelnakri
Copy link
Owner

Hi @marioimr, thanks for your contribution! I've received few request before about adding the initialize feature so I'm willing to approve this PR. I've reviewed your PR, however we need test cases as well. Could you add them to this PR @marioimr ?

@marioimr
Copy link
Contributor Author

Hi @marioimr, thanks for your contribution! I've received few request before about adding the initialize feature so I'm willing to approve this PR. I've reviewed your PR, however we need test cases as well. Could you add them to this PR @marioimr ?

Hi @izelnakri, absolutely in the next days I'll add tests as well. Thank you!

@izelnakri
Copy link
Owner

Hi @marioimr , I'm willing to merge this PR once the asked changes and tests are added. Thanks!

mayel added a commit to bonfire-networks/paper_trail that referenced this pull request Jun 26, 2023
@mayel mayel mentioned this pull request Jun 26, 2023
@izelnakri
Copy link
Owner

Hi @marioimr, This issue should be resolved in #224 . Feel free to open a new PR if it misses a certain thing.

@izelnakri izelnakri closed this Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants