-
Notifications
You must be signed in to change notification settings - Fork 360
Fix legacy redirection of legacy mainnet links #3298
Conversation
CLA Assistant Lite All Contributors have signed the CLA. |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
Pull Request Test Coverage Report for Build 1693422458
💛 - Coveralls |
Deployment links
|
E2E Tests Failed Failed tests:
|
rinkeby: 'rin', | ||
volta: 'vt', | ||
xdai: 'xdai', | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove that as this redirect will be done on the server side.
const chain = getChains().find((chain) => sameString(chain.chainName, chainLabel)) || getChainInfo() | ||
const subdomain = window.location.hostname.split('.')[0] | ||
|
||
const DEFAULT_SHORT_NAME = DEFAULT_CHAIN_ID === CHAIN_ID.RINKEBY ? LEGACY_ROUTE_SHORT_NAMES.rinkeby : 'eth' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please put rin and eth into an enum.
AFAIU, we need to hardcode them in this instance because the chains aren't loaded yet.
Or maybe they are already present in the codebase or in the Gateway SDK?
Question. The only redirection always goes to "rin:". I assume because this is a PR, and in prod, when the URL is something like "https://xdai.gnosis-safe.io/app/#/" it will redirect to the proper xdai: network. Is this correct? If this is the case then the ticket is good to be merged |
Yes, the redirection that already exists on the old subdomains redirects correctly but because of the dynamic loading of configs from the CGW and renaming of Mainnet to Ethereum, it is causing problems with finding the DEFAULT_CHAIN_ID === CHAIN_ID.RINKEBY ? SHORT_NAME.RINKEBY : SHORT_NAME.MAINNET I will merge this now. |
What it solves
Broken legacy redirection of mainnet.
How this PR fixes it
By loading chains from backend, the config is not immediately present for the redirection logic to reference. The default
shortName
now corresponds to the default network.How to test it
{{PR}}/app/#/safes/0xfF501B324DC6d78dC9F983f140B9211c3EdB4dc7/balances
{{PR}}/app/eth:0xfF501B324DC6d78dC9F983f140B9211c3EdB4dc7/balances
because the PR has no subdomain from the map.