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

feat: override org name + photo #4646

Merged
merged 6 commits into from
Jun 24, 2019
Merged

feat: override org name + photo #4646

merged 6 commits into from
Jun 24, 2019

Conversation

thelostone-mc
Copy link
Member

@thelostone-mc thelostone-mc commented Jun 17, 2019

Description

  • updated models Profile & Bounty -> override name + url
  • added provision in admin to enable admin overrides
  • updated template to use newly added property
  • update existing prop to use override name + url

Steps

In this example -> assume we'd be updating metamask name to xyz and their logo

Update Profile (/profile/xyz)
  • head over to admin profile section and click on profile you need to update
  • update Admin override name to the new name (xyz)
  • upload photo Admin override avatar:

^ This would ensure that the profile page of the org shows the overridden name + photo

Update Bounty (only for bounty + hackathon)

This is the tricky part.

  • You'll have to select all the bounties created by the org
  • For each bounty -> navigate to the bounty admin
  • update Admin override org name to the new name (xyz)
  • upload photo Admin override org logo

^ This would ensure that the bounty details page of the org shows the overridden name + photo
These are temp fixes which will be removed out once orgs is built out as a feature

note: do not use this for non-hackathon bounties ( especially name change as the filters extra map to the actual Github name)

Demo: Check HERE

@codecov
Copy link

codecov bot commented Jun 17, 2019

Codecov Report

Merging #4646 into master will increase coverage by <.01%.
The diff coverage is 22.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4646      +/-   ##
==========================================
+ Coverage   30.06%   30.06%   +<.01%     
==========================================
  Files         213      213              
  Lines       17098    17113      +15     
  Branches     2311     2317       +6     
==========================================
+ Hits         5140     5145       +5     
- Misses      11758    11766       +8     
- Partials      200      202       +2
Impacted Files Coverage Δ
app/dashboard/admin.py 65.55% <0%> (-0.37%) ⬇️
app/dashboard/models.py 56.46% <26.31%> (-0.18%) ⬇️

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 cdb2486...131b268. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 17, 2019

Codecov Report

Merging #4646 into master will decrease coverage by <.01%.
The diff coverage is 20%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4646      +/-   ##
==========================================
- Coverage   30.43%   30.43%   -0.01%     
==========================================
  Files         216      216              
  Lines       17188    17210      +22     
  Branches     2322     2329       +7     
==========================================
+ Hits         5232     5238       +6     
- Misses      11749    11764      +15     
- Partials      207      208       +1
Impacted Files Coverage Δ
app/dashboard/helpers.py 13.96% <ø> (ø) ⬆️
app/dashboard/views.py 14.36% <0%> (-0.05%) ⬇️
app/dashboard/admin.py 65.55% <0%> (-0.37%) ⬇️
app/dashboard/models.py 55.68% <27.27%> (-0.21%) ⬇️

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 3649843...17503e5. Read the comment docs.

- updated models Profile & Bounty -> override name + url
- added provision in admin to enable admin overrides
- updated template to use newly added property
- update existing prop to use override name + url
@thelostone-mc thelostone-mc requested review from alexvotofuture and a team June 18, 2019 16:05
@thelostone-mc thelostone-mc marked this pull request as ready for review June 18, 2019 16:05
@danlipert
Copy link
Contributor

Very nice! I think the URL issue is okay since its based on their github name, so its not "wrong" per se, just not perfect - and changing it would be a big headache. @vs77bb @alexvotofuture what do y'all think?

danlipert
danlipert previously approved these changes Jun 19, 2019
Copy link
Contributor

@danlipert danlipert left a comment

Choose a reason for hiding this comment

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

Looks good, just a few questions but otherwise approved 👍 nice!

app/dashboard/models.py Show resolved Hide resolved
app/dashboard/models.py Show resolved Hide resolved
@thelostone-mc
Copy link
Member Author

@gitcoinco/engineers So look like the hackathon event association does get ported over in merge_bounty 🤷‍♂
False alarm! Added the overridden fields there and tested it out.
Overrides stay on blockchain interaction steps

Screenshot 2019-06-19 at 10 26 34 PM

@thelostone-mc thelostone-mc requested a review from octavioamu June 19, 2019 17:00
octavioamu
octavioamu previously approved these changes Jun 19, 2019
Copy link
Contributor

@octavioamu octavioamu left a comment

Choose a reason for hiding this comment

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

LGTM

danlipert
danlipert previously approved these changes Jun 20, 2019
@danlipert
Copy link
Contributor

@thelostone-mc we'll need to rename the migration to 0035 and fix, I have one for dashboard as well (lots of migrations this deploy)

@thelostone-mc thelostone-mc dismissed stale reviews from danlipert and octavioamu via f9ad9b8 June 24, 2019 17:17
octavioamu
octavioamu previously approved these changes Jun 24, 2019
@thelostone-mc thelostone-mc merged commit d21f61f into master Jun 24, 2019
thelostone-mc added a commit that referenced this pull request Jun 26, 2019
@thelostone-mc thelostone-mc deleted the override branch July 4, 2019 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants