Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add papertrail gem #587
base: master
Are you sure you want to change the base?
Add papertrail gem #587
Changes from all commits
e98d484
8db48ad
9317dcd
5cd488a
e9d37a3
045b832
7983e14
e20bd83
7f50548
8775a62
5c9f03a
96a63c0
610a59b
f33f40f
2473cb2
acfc099
d73cae2
3744505
c135350
f9efbb4
f265b72
ad72818
590be35
cf7c8b7
d2a96b3
d744841
9ae217a
6be41fe
739a20f
5a064b1
fbe53cf
ab7e6cb
00dd3e1
477125a
5b61e29
8508e8a
1b63d3a
04d1424
cc89947
1017f8c
133b5a5
cb57571
6ac6a91
d537036
b88a00e
e98beef
1417b36
5c1ac4b
df3a9c9
5aec2e0
b991304
0de70d1
a1aabdd
51c8933
23fd312
f79ad57
c06353f
6d84e7f
b355ea1
e055936
5b352a9
37a0ba0
3286250
143eb24
791a15e
25937f6
93aebfb
30b9714
77d08a7
d478e36
c56df1a
679ea3f
17bb5d9
f439030
b737cbc
58f413c
f19dc36
8402150
c2096e0
f21bbbf
94f42b4
b4a0fa5
8aa4ddc
93dc302
41e728c
67661b0
1217ef4
ee2bc92
8c7782a
65b5d3d
7923d78
0b45274
f3a4e62
1ad9130
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is that necessary? (not necessarily criticizing just trying to understand)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Robin481 The papertrail versions are not deleted from the db by default, if the encryptable they correspond to is deleted. We discussed if it would make sense to keep the old versions because the api user may still want to know what happened to an encryptable that doesn't exist anymore. However, keeping the old encryptables may pollute the db quite heavily. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes deleting them makes sense.
However,
dependent: :destroy
should already do that, that is why I am asking. 😁(there was more text here that I deleted because I got confused by which branch i was on 😂 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Robin481 Yes,
dependent: :destroy
should do that but it doesn't. There might be something wrong with the way we used active record with regards topaper_trail
. I look into it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update on this front?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems to be a bug within the papertrail gem,
dependent: :destroy
does not work here but I extended the encryptable_spec to make sure that our:destroy versions
method deletes everything it need to and nothing more so I think we should leave it like this