fix: allow end user to set all url paths (WPB-4676) #16774

V-Gira · 2024-02-06T17:39:27Z

WPB-4676 URL_PATHS in webapp have inconsistencies and deprecations

Description

get rid of unnecessary logic building urls in externatRoutes.ts
avoid using hard coded support urls and subpaths
change the release note url from github to medium

Checklist

PR has been self reviewed by the author;
Hard-to-understand areas of the code have been commented;
If it is a core feature, unit tests have been added;

codecov · 2024-02-06T18:46:06Z

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (393c5df) 45.84% compared to head (6114793) 45.85%.
Report is 5 commits behind head on dev.

Additional details and impacted files

@@           Coverage Diff           @@
##              dev   #16774   +/-   ##
=======================================
  Coverage   45.84%   45.85%           
=======================================
  Files         747      747           
  Lines       24443    24442    -1     
  Branches     5578     5578           
=======================================
  Hits        11207    11207           
+ Misses      11818    11812    -6     
- Partials     1418     1423    +5

V-Gira · 2024-02-07T14:19:36Z

src/script/externalRoute.ts

-export const URL_PATH = {
-  CREATE_TEAM: '/create-team/',
-  DECRYPT_ERROR_1: '/articles/207948115',
-  DECRYPT_ERROR_2: '/privacy/error-2/',
-  MANAGE_SERVICES: '/services/',
-  MANAGE_TEAM: '/login/',
-  PASSWORD_RESET: '/forgot/',
-  PRIVACY_HOW: '/privacy/how/',
-  PRIVACY_UNVERIFIED_USERS: '/articles/202857164',
-  PRIVACY_WHY: '/articles/207859815',
-  SUPPORT_USERNAME: '/support/username/',
-} as const;


some of those are removed as deprecated

support links still in use have been moved with existing ones as environment variables instead of being hardcoded

url paths intended to be dynamically appended have been made into the URL_SUBPATH env variables as to be end user configurable

V-Gira · 2024-02-07T14:22:47Z

src/script/externalRoute.ts

-
-const addLocaleToHelpCenterUrl = (url?: string): string => {
-  if (!url) {
-    return undefined;
-  }
-  const language = currentLanguage().slice(0, 2);
-  const websiteLanguage = language == 'de' ? language : 'en-us';
-  return url.replace(
-    `${Config.getConfig().URL.SUPPORT.INDEX}`,
-    `${Config.getConfig().URL.SUPPORT.INDEX}/hc/${websiteLanguage}`,
-  );
-};


help center links do not need to be localized as they follow the browser locale setting

server/config/client.config.ts

CLAassistant · 2024-02-12T10:57:29Z

All committers have signed the CLA.

V-Gira · 2024-02-12T12:53:50Z

src/script/externalRoute.ts

+export const getManageServicesUrl = (utmSource?: string): string | undefined =>
+  getTeamSettingsUrl(URL.URL_PATH.MANAGE_SERVICES, utmSource);


hard to find doc on those UTM sources
we use 2 variations of those in the webapp:

client_landing when navigating from the main portion of the app

client_settings when navigating from the settings

Do we want to keep that granularity @atomrc ?
Should I had those to the externalUrl object and stop exporting those methods?

ah!! I do not know about those utms, honestly. But, since we will be tracking team settings usages soonish, I'm guessing those will be used at some point (if they are not currently).
We should probably keep a way to add utm to some urls I guess

I'll keep the export of the method and add some JSDoc to make the utmSource thing more transparent, how does that sound?

That sounds good to me :)

src/script/externalRoute.ts

przemvs · 2024-02-13T10:22:34Z

src/script/externalRoute.ts


-export const addLocaleToUrl = (url?: string): string => {
+export const addLocaleToUrl = (url?: string | undefined): string | undefined => {


Why url?: string | undefined? :D You append undefined by ? after url :)

Co-authored-by: Przemysław Jóźwik <[email protected]>

przemvs

LGTM! :)

atomrc

LGTM 😎

* fix: allow end user to set all url paths (WPB-4676) * add new urls in env variables to config * remove hard coded path from externalRoutes util * update components using the externalRoutes util * update missing components * remove unused path * rename subpath to url_path * expose external urls instead of get methods * add JS doc to methods appending UTM parameters * Apply suggestions from code review Co-authored-by: Przemysław Jóźwik <[email protected]> --------- Co-authored-by: Virgile <[email protected]> Co-authored-by: Przemysław Jóźwik <[email protected]>

* fix: allow end user to set all url paths (WPB-4676) (#16774) * fix: allow end user to set all url paths (WPB-4676) * add new urls in env variables to config * remove hard coded path from externalRoutes util * update components using the externalRoutes util * update missing components * remove unused path * rename subpath to url_path * expose external urls instead of get methods * add JS doc to methods appending UTM parameters * Apply suggestions from code review Co-authored-by: Przemysław Jóźwik <[email protected]> --------- Co-authored-by: Virgile <[email protected]> Co-authored-by: Przemysław Jóźwik <[email protected]> * chore: bump default config to 0.31.22 [WPB-10937] --------- Co-authored-by: Virgile <[email protected]> Co-authored-by: Przemysław Jóźwik <[email protected]>

otto-the-bot added type: bug / fix 🐞 👕 size: M labels Feb 6, 2024

V-Gira commented Feb 7, 2024

View reviewed changes

V-Gira marked this pull request as ready for review February 7, 2024 14:23

V-Gira requested review from otto-the-bot and a team as code owners February 7, 2024 14:23

atomrc reviewed Feb 8, 2024

View reviewed changes

server/config/client.config.ts Outdated Show resolved Hide resolved

PatrykBuniX approved these changes Feb 8, 2024

View reviewed changes

V-Gira and others added 7 commits February 12, 2024 12:21

fix: allow end user to set all url paths (WPB-4676)

3b56124

add new urls in env variables to config

bdb8746

remove hard coded path from externalRoutes util

32c950e

update components using the externalRoutes util

f17cd8c

update missing components

5849270

remove unused path

ecdc368

rename subpath to url_path

9f30b9c

V-Gira force-pushed the v/appended-paths-config branch from 3292d38 to 9f30b9c Compare February 12, 2024 11:21

expose external urls instead of get methods

ec60e06

V-Gira commented Feb 12, 2024

View reviewed changes

V-Gira added 2 commits February 12, 2024 14:20

add JS doc to methods appending UTM parameters

345e7a7

fix tests

ddf23d9

przemvs reviewed Feb 13, 2024

View reviewed changes

src/script/externalRoute.ts Outdated Show resolved Hide resolved

przemvs reviewed Feb 13, 2024

View reviewed changes

src/script/externalRoute.ts Outdated Show resolved Hide resolved

przemvs reviewed Feb 13, 2024

View reviewed changes

src/script/externalRoute.ts Outdated Show resolved Hide resolved

przemvs reviewed Feb 13, 2024

View reviewed changes

V-Gira and others added 2 commits February 13, 2024 11:31

Apply suggestions from code review

19eb6e0

Co-authored-by: Przemysław Jóźwik <[email protected]>

apply cr suggestions

6114793

V-Gira requested a review from atomrc February 13, 2024 10:44

V-Gira requested a review from przemvs February 13, 2024 10:44

przemvs approved these changes Feb 13, 2024

View reviewed changes

atomrc approved these changes Feb 13, 2024

View reviewed changes

V-Gira merged commit 3419ca2 into dev Feb 13, 2024
11 checks passed

V-Gira deleted the v/appended-paths-config branch February 13, 2024 11:39

V-Gira mentioned this pull request Sep 6, 2024

chore: allow support for appended URL paths [WPB-10937] #18013

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: allow end user to set all url paths (WPB-4676) #16774

fix: allow end user to set all url paths (WPB-4676) #16774

V-Gira commented Feb 6, 2024 •

edited by github-actions bot

Loading

codecov bot commented Feb 6, 2024 •

edited

Loading

V-Gira Feb 7, 2024 •

edited

Loading

V-Gira Feb 7, 2024

CLAassistant commented Feb 12, 2024 •

edited

Loading

V-Gira Feb 12, 2024

atomrc Feb 12, 2024

V-Gira Feb 12, 2024

atomrc Feb 12, 2024

przemvs Feb 13, 2024

przemvs left a comment

atomrc left a comment

		export const getManageServicesUrl = (utmSource?: string): string \| undefined =>
		getTeamSettingsUrl(URL.URL_PATH.MANAGE_SERVICES, utmSource);


		export const addLocaleToUrl = (url?: string): string => {
		export const addLocaleToUrl = (url?: string \| undefined): string \| undefined => {

fix: allow end user to set all url paths (WPB-4676) #16774

fix: allow end user to set all url paths (WPB-4676) #16774

Conversation

V-Gira commented Feb 6, 2024 • edited by github-actions bot Loading

Description

Checklist

codecov bot commented Feb 6, 2024 • edited Loading

Codecov Report

V-Gira Feb 7, 2024 • edited Loading

Choose a reason for hiding this comment

V-Gira Feb 7, 2024

Choose a reason for hiding this comment

CLAassistant commented Feb 12, 2024 • edited Loading

V-Gira Feb 12, 2024

Choose a reason for hiding this comment

atomrc Feb 12, 2024

Choose a reason for hiding this comment

V-Gira Feb 12, 2024

Choose a reason for hiding this comment

atomrc Feb 12, 2024

Choose a reason for hiding this comment

przemvs Feb 13, 2024

Choose a reason for hiding this comment

przemvs left a comment

Choose a reason for hiding this comment

atomrc left a comment

Choose a reason for hiding this comment

V-Gira commented Feb 6, 2024 •

edited by github-actions bot

Loading

codecov bot commented Feb 6, 2024 •

edited

Loading

V-Gira Feb 7, 2024 •

edited

Loading

CLAassistant commented Feb 12, 2024 •

edited

Loading