Spike use of presenters during serialization #950
Closed
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
solebared
force-pushed
the
spike-presenters-during-serialization
branch
from
9f3e9ff
to
8cd4b8a
Compare
This was referenced Apr 28, 2021
Merged
Experimenting with introducing presenters between controllers and serializers. Already liking the way this simplifies the `view_path` logic that was dispersed between the controller and serializer.
Instead of trying to split before parentheses. We believe this was intended to hide any extra details entered by the user, such as gender pronouns. But that format is not predictable, and this simplifies the code.
And have ContributionPresenter decorate person with this presenter.
solebared
force-pushed
the
spike-presenters-during-serialization
branch
from
8cd4b8a
to
c60af22
Compare
solebared
changed the base branch from
JadeDickinson-show-contributor-name
to
spike-presenters-during-serialization-base
11 tasks
|
Closing in favor of #953 . |
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.
Why
This is a spike on top of #917 (and #951, #952) experimenting with using Presenters to solve the problem of conditionally showing attributes based on authorization policies.
What, How
BasePresenterwith some functionality useful across presenterspresenthelper method for use in erb templatesPersonPolicythat allowed guest users access to any other Person w/o an associated userperson.nameon/contributionsby not trying to split the nameContributionPresenterand thread it into the serialization process inContributionsController#indexPersonPresenter, use it inContributionPresenterand have it conditionally show the name based onPersonPolicy.Probably easiest to review each commit individually.
Testing
Adds some specs for new code.
Next Steps
Outstanding Questions, Concerns and Other Notes
This solution works and feels like the right place to put logic.
However i wonder how it would scale when we need to conditionally show more than one attribute. Feels like we may need something analogous to
permitted_attributeswhich lists all attributes allowed to be submitted, but in this case, a listing of attributes allowed to be shown. Maybe avisible_attributesmethod on the policy that could be used by the presenter to dynamically mask any disallowed attributes?Pre-Merge Checklist