Addons-frontend: Use explicit regex for accepted add-on types in AMO routes

Created on 3 May 2017  路  11Comments  路  Source: mozilla/addons-frontend

Once we get the new router (which will probably happen in https://github.com/mozilla/addons-frontend/issues/2161) then we can change all of these routes:

<Route path=":visibleAddonType/categories/" component={CategoryList} />

to a specific regex list of accepted types, like:

<Route path=":visibleAddonType(addon|theme|...)/categories/" component={CategoryList} />

This will prevent confusing 500 errors when you type in a URL like /en-US/firefox/users/register/.

code quality assigned welcome p3 verified fixed triaged

All 11 comments

We can also remove error handling code in the component for 404s

This should be doable once #5652 lands.

Hey guys 馃憢馃徑I'd like to help with this if it's still available!

@SeanPrashad sure! This issue is about:

  • adding a regexp in the route's path of the category pages
  • removing the code that validates the visibleAddonType parameter (404, etc.)

Ok, I've made some progress and would like to make sure I'm not heading down the wrong 馃惏hole @willdurand:

All of the <Route> components from React live in src/amo/components/Routes/index.js.

Three occurrences of visibleAddonType appear as path params:

  1. https://github.com/mozilla/addons-frontend/blob/43ee8c787128f14fabc87b58287f097bc6ecd405/src/amo/components/Routes/index.js#L92-L96

  2. https://github.com/mozilla/addons-frontend/blob/43ee8c787128f14fabc87b58287f097bc6ecd405/src/amo/components/Routes/index.js#L97-L101

  3. https://github.com/mozilla/addons-frontend/blob/43ee8c787128f14fabc87b58287f097bc6ecd405/src/amo/components/Routes/index.js#L144-L148

I figured that finding the list of acceptable addon types would be the next move. visibleAddonType turns out to be a function, which is found here:

https://github.com/mozilla/addons-frontend/blob/43ee8c787128f14fabc87b58287f097bc6ecd405/src/core/utils/index.js#L193-L205

Within the definition of visibleAddonType, I saw VISIBLE_ADDON_TYPES_MAPPING which tells me that it's being used as a lookup for a set of values:

https://github.com/mozilla/addons-frontend/blob/43ee8c787128f14fabc87b58287f097bc6ecd405/src/core/constants.js#L68-L75

Now we'll have to figure out the right regex that will conform to all 7 valid types. I'm not sure where to place the hardcoded expression and where we'll need to do something like .match().

Is there anything I missed along the way that might give me some clues?

I believe only "themes" or "extensions" are used:

export const API_ADDON_TYPES_MAPPING = {
  extensions: ADDON_TYPE_EXTENSION,
  themes: ADDON_TYPE_THEME,
};

You can add the values in the path prop of each route.

Gotcha - for reference, the Route components path prop uses path-to-regexp.

@willdurand Please add some STR for qa or mark it "qa not needed".
Thanks!

@ioanarusiczki sorry, please check that we can still navigate to the landing pages (extensions, themes) and category pages (including the android page with the list of all categories).

Verified on AMO dev with FF63 - Win10 & Android 8.0
The detail pages for extensions/themes are displayed. I also checked that the list of categories is displayed and then verified each category on Windows and then on Android.

Was this page helpful?
0 / 5 - 0 ratings