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.

Sign up for GitHub

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

Jump to bottom

Jade dickinson show contributor name #917

Merged
merged 16 commits into from
Jun 18, 2021
Merged

Jade dickinson show contributor name #917

merged 16 commits into from
Jun 18, 2021

Conversation

svileshina
Copy link
Collaborator

@svileshina svileshina commented Mar 26, 2021 •
edited
Loading

Why

This adds the contributor's name to a contribution - Working off of #791

What

Added the contributor name to contributions through contribution.person, so added person to the contribution blueprint and appropriate tests and supporting files

Next Steps

Outstanding Questions, Concerns and Other Notes

We would like to replace raw json (i.e. lib/contributions.json) in the future

Meta

We added Jade as a contributor because this was based on their work (#791) and closed their original PR that had merge conflicts

Pre-Merge Checklist

  • Security & accessibility have been considered
  • Tests have been added, or an explanation has been given why the features cannot be tested
  • Documentation and comments have been added to the codebase where required
  • Entry added to CHANGELOG.md if appropriate
  • Outstanding questions and concerns have been resolved
  • Any next steps have been turned into Issues or Discussions as appropriate

JadeDickinson and others added 6 commits October 18, 2020 10:06
@JadeDickinson
Show contributor name on List view of Contributions page
c1c4c80
@JadeDickinson
Show contributor name on Tile view of Contributions page
46351bf
@svileshina @maebeale
wip resolving conflicts
161aefd 
Co-authored-by: maebeale <maebeale@gmail.com>
@svileshina
correctly render json in contribution blueprint spec
db607c9 
Co-authored-by: maebeale <maebeale@gmail.com>
@svileshina
added person to contributions.json to fix ListBrowser.vue spec
1abf5b9 
Co-authored-by: connieh1 <connie2630a@gmail.com>
Co-authored-by: JadeDickinson <14929975+JadeDickinson@users.noreply.github.com>
Co-authored-by: maebeale <maebeale@gmail.com>
@svileshina
Merge branch 'main' into JadeDickinson-show-contributor-name
901fb64
@svileshina
Copy link
Collaborator Author

svileshina commented Apr 10, 2021 •
edited
Loading

we are going to wait on for the "Add a conditional so this is only displayed for logged in users with admin permissions" to be done before potentially merging @maebeale #933

This was referenced Apr 10, 2021
Closed
Closed
@svileshina
Copy link
Collaborator Author

Some thoughts/questions we had in trying to make this conditional:

  • we can keep pronouns (get rid of split)
  • we are want to move conditional into blueprinter which requires figuring out dynamic selection of which view to render (similar "Supporting Dynamic Blueprints For Associations" of blueprinter... perhaps)
  • alternative: just put the conditional and name into a presenter which then gets passed to serializer

@JadeDickinson JadeDickinson mentioned this pull request Apr 17, 2021
Merged
9 tasks
@solebared solebared mentioned this pull request Apr 24, 2021
Closed
14 tasks
@solebared
Merge branch 'main' into JadeDickinson-show-contributor-name
a844189
@solebared
Merge branch 'main' into JadeDickinson-show-contributor-name
d6d5627
@solebared
Show Person's full name on /contributions
a8fa7b7 
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.
@solebared
Fix some rubocop violations surfaced from merging main
30de713
@solebared
Merge branch 'main' into JadeDickinson-show-contributor-name
7871bf2 
Conflicts:
      lib/contributions.json
      spec/blueprints/contribution_blueprint_spec.rb
@solebared
Merge branch 'main' into JadeDickinson-show-contributor-name
b3bf22d
@solebared
Require current_user when serializing with BlueprintAuthorization
3ee05a3 
And provide it in `contributions_controller`.
@solebared
Conditionally hide contributor name column in ListBrowser.vue
6fc8030 
Note: the testing pattern used in ListBrowser.spec.js mutates fixture
data which is asking for trouble. I've stuck with it in this commit and
left a fixme to come back around and clean it up.
@solebared
Conditionally show contributor name in TileListItem
3c72d76
@solebared solebared force-pushed the JadeDickinson-show-contributor-name branch from a21f987 to 3c72d76 Compare June 14, 2021 21:19
@solebared
Copy link
Collaborator

@svileshina , please take a look at my 3 most recent commits. I believe this completes the outstanding issues on this branch?

@solebared solebared marked this pull request as ready for review June 18, 2021 00:44
solebared
solebared approved these changes Jun 18, 2021
View reviewed changes
Copy link
Collaborator

@solebared solebared left a comment

Choose a reason for hiding this comment

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

Reviewed in person with @svileshina .

@svileshina
Copy link
Collaborator Author

looks great, thank you for all the work!!!

@solebared solebared merged commit 5459510 into main Jun 18, 2021
@solebared solebared deleted the JadeDickinson-show-contributor-name branch June 18, 2021 00:45
@solebared solebared mentioned this pull request Dec 20, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@solebared solebared solebared approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@svileshina @solebared @JadeDickinson