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

Update /certified paths to favor pretty links #13051

Merged
merged 1 commit into from
Jul 29, 2023

Conversation

carkod
Copy link
Contributor

@carkod carkod commented Jul 25, 2023

Done

At the moment https://ubuntu.com/certified?q=&category=Desktop and Certified desktops | Ubuntu look different. They shouldn’t. The former (the regular search page) should redirect to the latter (the category page).

Pretty URLs should take precedence, so queries like https://ubuntu.com/certified?q=Nvidia&category=Desktop should be redirected to https://ubuntu.com/certified/desktops?q=Nvidia (URL should remain unchanged when more than one category is selected)

When there is more than one category selected, URL structure should remain unchanged: https://ubuntu.com/certified?q=Nvidia&category=Desktop&category=Laptops

QA

Issue / Card

Fixes https://warthogs.atlassian.net/browse/WD-4665?atlOrigin=eyJpIjoiMDhiYTUyZDkwYWI0NGQxZWJkOTA1MWRlMWVjYTk3YTMiLCJwIjoiaiJ9

Screenshots

[If relevant, please include a screenshot.]

Help

QA steps - Commit guidelines

@webteam-app
Copy link

Demo starting at https://ubuntu-com-13051.demos.haus

@carkod carkod changed the title WIP Update /certified paths to favor pretty links Update /certified paths to favor pretty links Jul 25, 2023
@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #13051 (cef07c2) into main (76281b1) will decrease coverage by 0.11%.
Report is 47 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main   #13051      +/-   ##
==========================================
- Coverage   74.90%   74.80%   -0.11%     
==========================================
  Files         107      107              
  Lines        2873     2881       +8     
  Branches      928      936       +8     
==========================================
+ Hits         2152     2155       +3     
- Misses        699      704       +5     
  Partials       22       22              

see 1 file with indirect coverage changes

@juanruitina
Copy link
Contributor

This throws a 404.

I have also noticed when more than one filter is selected, only one is preserved during the redirection. E.g. https://ubuntu-com-13051.demos.haus/certified?q=&category=Device&vendor=AAEON+Technology+Inc.&vendor=ADLink+Technology%2C+Inc. redirects to https://ubuntu-com-13051.demos.haus/certified/iot?q=&vendor=AAEON+Technology+Inc., dropping the second vendor filter. This also happens with the release filter. Can you address this?

@carkod carkod force-pushed the certified-filters-url-wd-4665 branch 2 times, most recently from 0e17066 to 5e3dea3 Compare July 26, 2023 17:16
@carkod
Copy link
Contributor Author

carkod commented Jul 26, 2023

This throws a 404.

I have also noticed when more than one filter is selected, only one is preserved during the redirection. E.g. https://ubuntu-com-13051.demos.haus/certified?q=&category=Device&vendor=AAEON+Technology+Inc.&vendor=ADLink+Technology%2C+Inc. redirects to https://ubuntu-com-13051.demos.haus/certified/iot?q=&vendor=AAEON+Technology+Inc., dropping the second vendor filter. This also happens with the release filter. Can you address this?

  • Sorry the 404 was because I pasted the wrong link.
  • The multiple vendor selection should be fixed now.

@juanruitina
Copy link
Contributor

LGTM!

Copy link
Contributor

@pedoch pedoch left a comment

Choose a reason for hiding this comment

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

LGTM!

webapp/certified/views.py Outdated Show resolved Hide resolved
Redirect search ?category=<category-name> to /<category-name> except
when multiple categories are selected.
@carkod carkod force-pushed the certified-filters-url-wd-4665 branch from 5e3dea3 to cef07c2 Compare July 28, 2023 23:56
@carkod carkod merged commit 37e254b into canonical:main Jul 29, 2023
15 of 16 checks passed
@carkod carkod deleted the certified-filters-url-wd-4665 branch July 29, 2023 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants