-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
AppCenterDistribute v3 task: add destination type and silent release options #9759
AppCenterDistribute v3 task: add destination type and silent release options #9759
Conversation
"loc.input.help.releaseNotesFile": "Select a UTF-8 encoded text file which contains the Release Notes for this version.", | ||
"loc.input.label.isMandatory": "Require users to update to this release", | ||
"loc.input.label.destinationType": "Release destination", | ||
"loc.input.label.destinationGroupIds": "Destination IDs", |
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.
Now that we select the destination type, can we make these more explicit? For example, this should be Distribution Group IDs
. And Destination ID
should be Store ID
?
When we add Tester support at some point, we can add Tester Emails
Would be good to collect some PM feedbacks here.
publishBody = Object.assign(publishBody, { build: build }); | ||
} | ||
|
||
request.patch({ url: publishReleaseUrl, headers: headers, json: publishBody }, (err, res, body) => { |
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.
Is this now using the new Distribution API (which differentiates between group, store and tester), or is this still the old one? For V3, now that we have destination type, we should move to the new one.
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.
No, it still uses old api. Thanks for note, I will switch to new one
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.
Updated to use new post api for publishing to destination (1 api call per destination)
Also added additional api call to set release notes, build etc
const destinationsInputName = destinationType === DestinationType.Groups ? 'destinationGroupIds' : 'destinationStoreId'; | ||
const destinationIsMandatory = destinationType === DestinationType.Store; | ||
|
||
let destinations = tl.getInput(destinationsInputName, destinationIsMandatory) || "00000000-0000-0000-0000-000000000000"; |
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.
It should only default to "00000000-0000-0000-0000-000000000000" when it is Distribution Group (the collaborator group)
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.
Yes. In case of store destination second parameter of getInput
is true so it will throw an error if destination is not provided
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.
As you can see from the screenshot it can't be saved with empty destination for store
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.
Got it. You probably want it to be reversed though (change destinationIsMandatory
to destinationIsOptional
). If we add tester support, then only "group" has default
|
||
let isMandatory: boolean = tl.getBoolInput('isMandatory', false); | ||
|
||
const destinationType = tl.getInput('destinationType', false) as DestinationType || DestinationType.Groups; |
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.
What's the point of having a default (DestinationType,.Groups
) in here? I thought this input is required?
const destinationsInputName = destinationType === DestinationType.Groups ? 'destinationGroupIds' : 'destinationStoreId'; | ||
const destinationIsMandatory = destinationType === DestinationType.Store; | ||
|
||
let destinations = tl.getInput(destinationsInputName, destinationIsMandatory) || "00000000-0000-0000-0000-000000000000"; |
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.
Got it. You probably want it to be reversed though (change destinationIsMandatory
to destinationIsOptional
). If we add tester support, then only "group" has default
Change ui strings to be more specific
Multiple Groups destinations and silent release option:
Store destination: