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

feat: envis for txd and checkout-service URLs #580

Merged
merged 6 commits into from
Oct 23, 2024

Conversation

kilted-andres
Copy link
Contributor

@kilted-andres kilted-andres commented Sep 23, 2024

fixes https://github.com/KILTprotocol/ticket/issues/3614

The TXD and Checkout Service URLs used to be mapped from the blockchain Endpoint. Recently, new endpoints for Spiritnet where added, but the maps where not updated for it. This was leading to bad HTTP requests and breaking the UI of production.

I now changed the approach to find out which TXD and Checkout Service URLs to use. So not all endpoints need to be explicitly listed on the maps, allowing newer endpoints to be used without changing code.

For my new approach I assume that peregrine endpoints include "peregrine" and peregrine-staging endpoints include "peregrine-stg" on their URIs. This is still way less assumptions than before, where the whole endpoint URI had to match.

The URLs for TxD and the checkout service need to be passed as environment variables now.

Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-580.dwo3t819w2m3x.amplifyapp.com

'wss://peregrine.kilt.io': 'https://dev.checkout.kilt.io',
'wss://peregrine-stg.kilt.io/para': 'https://smoke.checkout.kilt.io',
};
function deductTxdURL(blockchainEndpoint: string) {

Choose a reason for hiding this comment

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

I don't see why we would change it to this?

I think we should move these links into the environment variables outside of the application itself.

Once we handle them outside, we can change them without creating a PR.

That would mean this needs to be reworked to take an env

Choose a reason for hiding this comment

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

@ggera What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, did it.

New environment variables:

  • REACT_APP_CHECKOUT_URL
  • REACT_APP_TXD_URL

'wss://peregrine.kilt.io': 'https://dev.txd.trusted-entity.io',
'wss://peregrine-stg.kilt.io/para': 'https://smoke.txd.trusted-entity.io',
};
export const checkoutServiceURL = process.env.REACT_APP_CHECKOUT_URL as string;

Choose a reason for hiding this comment

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

Do we use different env for the different deployments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should.

Choose a reason for hiding this comment

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

@ggera Can you confirm?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unable to override a variable on Amplify at the moment. HODL

Choose a reason for hiding this comment

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

Looks good

@kilted-andres kilted-andres changed the title feat: functions to decide which txd and checkout-service to use feat: envis for txd and checkout-service URLs Oct 16, 2024
Copy link

@Dudleyneedham Dudleyneedham left a comment

Choose a reason for hiding this comment

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

LGTM. @ggera can you confirm you were able to override the variables on amp?

'wss://peregrine.kilt.io': 'https://dev.txd.trusted-entity.io',
'wss://peregrine-stg.kilt.io/para': 'https://smoke.txd.trusted-entity.io',
};
export const checkoutServiceURL = process.env.REACT_APP_CHECKOUT_URL as string;

Choose a reason for hiding this comment

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

Looks good

Copy link
Contributor

@Ad96el Ad96el left a comment

Choose a reason for hiding this comment

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

🚀

@ggera
Copy link
Contributor

ggera commented Oct 23, 2024

LGTM. @ggera can you confirm you were able to override the variables on amp?

yes it's possible now

@kilted-andres kilted-andres merged commit 82801b1 into main Oct 23, 2024
3 checks passed
@kilted-andres kilted-andres deleted the xw/stop_ibp_from_breaking_stuff branch October 23, 2024 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants