Replace internal comments for valuations thread #1179
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
Why: Budget Investment's valuators need to be able to comment on investments without making those comments public. We need a way to clearly make a distinction to avoid "leaking" internal valuation comments. How: Adding a boolean `valuation` attribute defaulted to false to the Comments table, and index on it with concurrent algorithm as explained at https://robots.thoughtbot.com/how-to-create-postgres-indexes-concurrently-in The name `valuation` was chosen instead of `internal` because of the more specific meaning as well as avoiding a collision with existing internal_comments attribute on Budget::Investment model (soon to be deprecated & removed)
Why: Internal valuation comments are only for admins and valuators, not for the public view. How: Adding a `not_valuations` scope and use it at the `public_for_api` one
Why: Budget Investments already has an existing `comments` relation that is on use. We need to keep that relation unaltered after adding the internal valuation comments, that means scoping the relation to only public comments (non valuation ones) so existing code using it will remain working as expected. A new second relation will be needed to explicitly ask for valuation comments only where needed, again scoping to valuation comments. How: Adding a second `valuations` relationship and filtering on both with the new `valuation` flag from Comment model.
Why: Budget Investment's valuators should be able to see internal valuation comments thread at both show and edit views. How: At Valuation::BudgetInvestmentsController: * Include CommentableActions to gain access to the entire feature, with required resource_model & resource_name methods. * Add the only possible order (oldest to newest) * Load comments on both show & edit actions, passing `valuations` flag to the CommentTree in order to only list those. At CommentTree: * Use `valuations` flag as instance variable to decide wich comment threat to load: valuations (if relation exists) or comments.
How: Using a local variable at partials to set a hidden true/false value for `valuation` parameter on the comment creation form. Allowing that new param at the comment controller and using it when building a new Comment.
Why: Only admins or valuators (for those investments they've assigned) can create internal valuation comments on them. How: * Creating a new `comment_valuation` ability for admins and valuators in the same manner the `valuate` ability works. * Adding a validation at Comment model for those with `valuation` flag active that checks if the author can make a valuation comment on the commentable, as well as the respective active record error messages. This will prevent comments from being created at a controller level as well. * Improving comment factory trait `valuation` to have an associated investment, author that is a valuator and setting the valuator on the valuators list of the investment
As Budget::Investment has two relationships over commentable polymorphic relationship, the counter_cache is counting the sum of both comments and valuations. We don't show valuations count anywhere, only the (public) comments so we just use comments.count in this case
Check that on both comment creation and comment reply the public facing investment page doesn't show them
Its more descriptive in the contexts where its rendered
It's going to be used at valuation and admin panels
d4dc8a4 to
c9e238a
Compare
b9802a6 to
2376b79
Compare
Merged
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.
Where
This PR merges two consul PR's in one
Resulting in the replacement of the internal comments for the valuation comments
What & How & Screenshots & Test
Please check both PR individually, descriptions are HUGE and very explicit
Deployment
Remember to run
bin/rake investments:internal_comments:migrate_to_thread RAILS_ENV=preproductionafter deploying to PRE andbin/rake investments:internal_comments:migrate_to_thread RAILS_ENV=productionto PRO environments!Warnings