Skip to content

Comments

Fixes app external URLs#57

Merged
kzuraw merged 1 commit intomainfrom
fix-app-external-urls
Aug 6, 2025
Merged

Fixes app external URLs#57
kzuraw merged 1 commit intomainfrom
fix-app-external-urls

Conversation

@kzuraw
Copy link
Contributor

@kzuraw kzuraw commented Jul 31, 2025

This PR fixes url generated by app to always be with baseUrl with properly encoded app id

@kzuraw kzuraw requested a review from witoszekdev July 31, 2025 14:16
@vercel
Copy link

vercel bot commented Jul 31, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
dummy-payment-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 31, 2025 2:17pm

Comment on lines 10 to 13
private getAppBaseUrlAbsolute(appId: string, saleorApiUrl: string) {
const saleorDashboardUrl = saleorApiUrl.replace("/graphql/", "");
return `${saleorDashboardUrl}${this.getAppBaseUrlRelative(appId)}`;
}
Copy link
Member

Choose a reason for hiding this comment

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

issue: please remove it since it's no longer used

Suggested change
private getAppBaseUrlAbsolute(appId: string, saleorApiUrl: string) {
const saleorDashboardUrl = saleorApiUrl.replace("/graphql/", "");
return `${saleorDashboardUrl}${this.getAppBaseUrlRelative(appId)}`;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is used inside:

getTransactionDetailsUrl(transactionId: string) {
    const baseUrl = this.getAppBaseUrlAbsolute(this.authData.appId, this.authData.saleorApiUrl);
    return `${baseUrl}/transactions/${transactionId}`;
  }

const baseUrl = includeSaleorBaseUrl
? this.getAppBaseUrlAbsolute(this.authData.appId, this.authData.saleorApiUrl)
: this.getAppBaseUrlRelative(this.authData.appId);
getTransactionDetailsUrl(transactionId: string) {
Copy link
Member

Choose a reason for hiding this comment

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

question: is this correct? don't we need to add Saleor URL? If so then maybe we should a link to docs here 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

after this change we always add Saleor URL (via getAppBaseUrlAbsolute)

@kzuraw kzuraw requested a review from witoszekdev August 4, 2025 13:32
@kzuraw kzuraw merged commit f63c209 into main Aug 6, 2025
9 checks passed
@kzuraw kzuraw deleted the fix-app-external-urls branch August 6, 2025 07:06
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.

2 participants