Use audited to track changes in investments #3811
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
javierm
force-pushed
the
investment_changelog
branch
from
79b11fb
to
e3aa28b
Compare
javierm
force-pushed
the
investment_changelog
branch
2 times, most recently
from
8a9c974
to
a3693da
Compare
javierm
changed the title
javierm
force-pushed
the
investment_changelog
branch
from
a3693da
to
74b98e7
Compare
javierm
force-pushed
the
investment_changelog
branch
from
74b98e7
to
8661474
Compare
Since we were saving the same budget investments several times, once for every language, we were creating more than 1000 changelog entries. Assigning the language attributes when creating the investments generates less entries, making it easier to work with them.
If we validate the presence of the old value and the new value, changes in optional fields will not be stored if either the old value or the new value are blank.
Our manual implementation had a few issues. In particular, it didn't track changes related to associations, which became more of an issue when we made investments translatable. Using audited gives us more functionality while at the same time simplifies our code. However, it adds one more external dependency to our project. The reason for choosing audited over paper trail is audited seems to make it easier to handle associations.
The same way we do for milestones. We also make the code more consistent since the view was already in a separate folder.
Note the user interface could certainly be improved, as it doesn't show which languages have changed.
javierm
force-pushed
the
investment_changelog
branch
from
8661474
to
a792b68
Compare
The name of the changed field is translated, values are truncated so descriptions with thousands of character would make this table huge and impossible to read, dates are localized, and values like arrays and booleans are displayed properly.
Don't use <label> tags for things that are not labels, add a proper <title> for the page, add a back link, remove an unnecessary `inline-block` style for a header, localize dates and field names, ... The interface could be further improve: proper diffs for long texts, better separation between fields, ...
javierm
force-pushed
the
investment_changelog
branch
3 times, most recently
from
3155b29
to
77310f0
Compare
javierm
force-pushed
the
investment_changelog
branch
from
77310f0
to
9565d6a
Compare
smarques
pushed a commit
to venetochevogliamo/consul
that referenced
this pull request
Apr 29, 2020
Use audited to track changes in investments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
References
Background
Our current implementation of #3433 has a few issues:
selectedwere marked as changing fromttofand vice versa, instead oftrueandfalseObjectives
Visual changes
Before these changes:
After these changes:
Notes
After these changes, the implementation is still far from perfect, but it's much better than it was.