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.

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 default ordering users in user directory #4602

Merged

Conversation

5 participants
@rafalkowalski
Copy link
Contributor

commented Jun 9, 2019

Please kindly review
@octavioamu @thelostone-mc

Description

Shows your coworkers at the begining in user directory

image

Refers/Fixes

Fixes #4398

@codecov

This comment has been minimized.

Copy link

commented Jun 9, 2019

Codecov Report

Merging #4602 into master will increase coverage by <.01%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4602      +/-   ##
==========================================
+ Coverage   30.12%   30.13%   +<.01%     
==========================================
  Files         210      210              
  Lines       16911    16914       +3     
  Branches     2284     2286       +2     
==========================================
+ Hits         5095     5097       +2     
- Misses      11619    11620       +1     
  Partials      197      197
Impacted Files Coverage Δ
app/dashboard/views.py 14.4% <75%> (+0.12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b17efb5...4bdab39. Read the comment docs.

@codecov

This comment has been minimized.

Copy link

commented Jun 9, 2019

Codecov Report

Merging #4602 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4602   +/-   ##
=======================================
  Coverage   30.42%   30.42%           
=======================================
  Files         216      216           
  Lines       17234    17234           
  Branches     2333     2333           
=======================================
  Hits         5243     5243           
  Misses      11783    11783           
  Partials      208      208
Impacted Files Coverage Δ
app/dashboard/views.py 14.44% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 911d831...efbdcd9. Read the comment docs.

@thelostone-mc thelostone-mc requested a review from gitcoinco/engineers Jun 10, 2019

@octavioamu

This comment has been minimized.

Copy link
Collaborator

commented Jun 11, 2019

@rafalkowalski thanks for this I think this code is outdated in master since some fixes got into stable, tomorrow we will consolidate master and stable but maybe this code will need some changes to work with that.

@rafalkowalski

This comment has been minimized.

Copy link
Contributor Author

commented Jun 11, 2019

@octavioamu Should i rebase it with master?

@thelostone-mc

This comment has been minimized.

Copy link
Member

commented Jun 12, 2019

@rafalkowalski we made a hot patch fix on stable ! could you wait 12 hours before rebasing
we need to get those commits into master

@rafalkowalski rafalkowalski force-pushed the rafalkowalski:I4398/user-directory-order branch from 4bdab39 to 4d25121 Jun 12, 2019

@rafalkowalski

This comment has been minimized.

Copy link
Contributor Author

commented Jun 12, 2019

@octavioamu I've rebased it with master

@octavioamu

This comment has been minimized.

Copy link
Collaborator

commented Jun 13, 2019

Now can be rebased @rafalkowalski

@rafalkowalski

This comment has been minimized.

Copy link
Contributor Author

commented Jun 13, 2019

@danlipert

This comment has been minimized.

Copy link
Collaborator

commented Jun 19, 2019

@rafalkowalski Can you update the description to include the required fields? Thanks!

@rafalkowalski

This comment has been minimized.

Copy link
Contributor Author

commented Jun 19, 2019

@danlipert I've updated the description

@danlipert

This comment has been minimized.

Copy link
Collaborator

commented Jun 21, 2019

@rafalkowalski How was this tested? Can you include some screenshots or a video showing how the changes affect the current behavior? Thanks!

@thelostone-mc thelostone-mc requested review from danlipert and octavioamu Jun 22, 2019

@rafalkowalski rafalkowalski force-pushed the rafalkowalski:I4398/user-directory-order branch from de95c53 to 7feff02 Jun 22, 2019

@rafalkowalski

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2019

@danlipert @octavioamu
Screenshot at 2019-06-22 21-23-17

First and second profiles are people who has worked with me before.
Screenshot at 2019-06-22 23-54-57

@rafalkowalski rafalkowalski force-pushed the rafalkowalski:I4398/user-directory-order branch from 7feff02 to b63b285 Jun 22, 2019

@thelostone-mc thelostone-mc added this to Review-Pending in PR Review Board Jun 23, 2019

@danlipert
Copy link
Collaborator

left a comment

Looks good @rafalkowalski - in the future please make sure to include the "Testing" section in the PR description so its easy to find and review, this is where you can also add screenshots or a video explaining the changes. Also, try not to make arbitrary style/spacing changes like https://github.com/gitcoinco/web/pull/4602/files#diff-3e886cd5b50df8f0cabc86d33ca734e6L777

@rafalkowalski

This comment has been minimized.

Copy link
Contributor Author

commented Jun 28, 2019

@danlipert thanks ;)

danlipert added some commits Jul 3, 2019

@thelostone-mc thelostone-mc merged commit 4205223 into gitcoinco:master Jul 3, 2019

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

PR Review Board automation moved this from Review-Pending to Done Jul 3, 2019

@PixelantDesign

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

hooray!

@PixelantDesign PixelantDesign added this to Review in progress in Grants Recipients Bounties Jul 3, 2019

@PixelantDesign PixelantDesign moved this from Review in progress to Done in Grants Recipients Bounties Jul 3, 2019

@octavioamu

This comment has been minimized.

Copy link
Collaborator

commented Jul 10, 2019

@rafalkowalski I had tested this in master and I see some issues

  • As a funder, not showing first the hunter I had worked with.
  • As a funder, for the funder is showing 1 bounties completed and 0 for the contributor (should be the opposite)
  • As a funder, for the contributor I had worked with is showing previously_worked: false and should be true.

@PixelantDesign PixelantDesign added sprint 14 and removed sprint 13 labels Jul 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.