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.

Sign up for GitHub

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

Jump to bottom

Add callbackUrl option #25

Closed
wants to merge 7 commits into from
Closed

Add callbackUrl option #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
598c993
fix: do not use document id as job name filter
arthur-pinner Oct 9, 2023
67a6927
refactor: receive callbackUrl as option
arthur-pinner Oct 10, 2023
a7a7b33
fix: import translated content back
arthur-pinner Oct 10, 2023
324f0a0
refactor: use CustomParams type
arthur-pinner Oct 10, 2023
2d44cc7
docs: update docs
arthur-pinner Oct 10, 2023
98f3557
refactor: use callbackUrl as function
arthur-pinner Oct 10, 2023
77b403b
fix: remove locale from fileUri
arthur-pinner Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/01-advanced-configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ const customConfig = {
},
},
]
},

// add custom params for Smartling
customParams: {
// GET request that creates a callback to a URL when a file is 100% published for a locale.
// The callback gives the fileUri and locale with the format http[/s]://your.url?locale=xx-XX&fileUri=your.file.
callbackUrl: 'https://your-endpoint.here'
}
//adapter, baseLanguage, secretsNamespace, importTranslation, exportForTranslation should likely not be touched unless you very much want to customize your plugin.
} satisfies TranslationsTabConfigOptions
Expand Down
10 changes: 9 additions & 1 deletion src/adapter/createTask.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {authenticate, getHeaders, findExistingJob} from './helpers'
import {Adapter, Secrets} from 'sanity-translations-tab'
import {Adapter, Secrets, CustomParams} from 'sanity-translations-tab'

Check failure on line 2 in src/adapter/createTask.ts

View workflow job for this annotation

GitHub Actions / Lint & Build 

Module '"sanity-translations-tab"' has no exported member 'CustomParams'.
import {getTranslationTask} from './getTranslationTask'
import {Buffer} from 'buffer'

Expand Down Expand Up @@ -93,6 +93,7 @@
secrets: Secrets,
localeIds: string[],
accessToken: string,
customParams?: CustomParams,
//eslint-disable-next-line max-params
) => {
const {project, proxy} = secrets
Expand All @@ -109,6 +110,10 @@
formData.append('file', new Blob([htmlBuffer]), `${document.name}.html`)
localeIds.forEach((localeId) => formData.append('localeIdsToAuthorize[]', localeId))

if (customParams?.callbackUrl && typeof customParams?.callbackUrl === 'function') {
formData.append('callbackUrl', customParams?.callbackUrl(document))
}

return fetch(proxy, {
method: 'POST',
headers: getHeaders(url, accessToken),
Expand All @@ -122,6 +127,8 @@
localeIds: string[],
secrets: Secrets | null,
workflowUid?: string,
customParams?: CustomParams,
// eslint-disable-next-line max-params
) => {
if (!secrets?.project || !secrets?.secret || !secrets?.proxy) {
throw new Error(
Expand Down Expand Up @@ -151,6 +158,7 @@
secrets,
localeIds,
accessToken,
customParams,
)
//eslint-disable-next-line no-console -- for developer debugging
console.info('Upload status from Smartling: ', uploadFileRes)
Expand Down
2 changes: 1 addition & 1 deletion src/adapter/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const findExistingJob = async (
'The Smartling adapter requires a Smartling project identifier and a proxy URL. Please check your secrets document in this dataset, per the plugin documentation.',
)
}
const url = `https://api.smartling.com/jobs-api/v3/projects/${project}/jobs?jobName=${documentId}`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The jobName QS filter has been removed because it breaks if the job does not use the documentId as job's name in Smartling.

It fetches all jobs and filter by referenceNumber using the same logic.

Copy link
Member

Choose a reason for hiding this comment

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

hey arthur! i would prefer this stay in the plugin for backwards compatibility. we've only added the additional logic to include the reference number a month or so ago, so other users may have old jobs that do not have a reference number encoded. ideally, this would be removed with a major version change, because it's technically a breaking change for users with in-progress jobs.

there is additional logic in the following few lines to use either the file URI or the reference number as a "catch." if you've noticed breaking behavior with this logic, let me know, but i'd rather catch both cases.

const url = `https://api.smartling.com/jobs-api/v3/projects/${project}/jobs`
//first, try fetching from name resolution
let items = await fetch(proxy, {
headers: getHeaders(url, accessToken),
Expand Down
Loading