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

#4503 - Add canonical URL to Bounty Details template #4579

Merged
merged 4 commits into from
Jun 18, 2019

Conversation

proofoftom
Copy link
Contributor

@proofoftom proofoftom commented Jun 5, 2019

Description

Added a reverse lookup of the bounty which provides a canonical URL sans the standard bounties id (i.e. /issue/gitcoinco/web/12)

Refers/Fixes

Fixes #4503

Testing

Added tests for the bounty.canonical_url property

@codecov
Copy link

codecov bot commented Jun 5, 2019

Codecov Report

Merging #4579 into master will decrease coverage by <.01%.
The diff coverage is 16.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4579      +/-   ##
==========================================
- Coverage   30.12%   30.12%   -0.01%     
==========================================
  Files         210      210              
  Lines       16911    16917       +6     
  Branches     2284     2284              
==========================================
+ Hits         5095     5096       +1     
- Misses      11619    11624       +5     
  Partials      197      197
Impacted Files Coverage Δ
app/dashboard/views.py 14.27% <0%> (-0.02%) ⬇️
app/dashboard/models.py 56.51% <20%> (-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...05c1ef8. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 5, 2019

Codecov Report

Merging #4579 into master will decrease coverage by <.01%.
The diff coverage is 87.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4579      +/-   ##
==========================================
- Coverage   30.06%   30.05%   -0.01%     
==========================================
  Files         213      213              
  Lines       17098    17106       +8     
  Branches     2311     2311              
==========================================
+ Hits         5140     5141       +1     
- Misses      11758    11765       +7     
  Partials      200      200
Impacted Files Coverage Δ
app/dashboard/views.py 14.32% <0%> (-0.02%) ⬇️
app/dashboard/models.py 56.82% <100%> (+0.18%) ⬆️
app/dashboard/embed.py 28.16% <0%> (-3.45%) ⬇️

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 348cdfb...5facf6e. Read the comment docs.

@proofoftom
Copy link
Contributor Author

Actually (and I'm no SEO expert obviously), I'm thinking that the canonical urls need to be absolute (not relative). Will look at making them so.

@proofoftom proofoftom force-pushed the feature/canonical-url branch from 7be767d to 2a4d344 Compare June 11, 2019 17:25
@proofoftom
Copy link
Contributor Author

Ready for review

@proofoftom
Copy link
Contributor Author

@danlipert Any review or feedback would be appreciated, cheers!

@thelostone-mc thelostone-mc requested a review from a team June 17, 2019 06:07
Copy link
Member

@thelostone-mc thelostone-mc 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
Copy link
Contributor

@proofoftom very nice!

@danlipert danlipert requested a review from octavioamu June 18, 2019 13:20
@danlipert danlipert merged commit b8616c5 into gitcoinco:master Jun 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants