Skip to content

fix(appflow): Default should be all available not hard-coded by platform #4645

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

Merged
giocalitri merged 2 commits into from
Feb 18, 2021
Merged

fix(appflow): Default should be all available not hard-coded by platform #4645

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b515c2
fix(appflow): Default should be all available not hard-coded by platform
jlane9 Feb 17, 2021
7ad189a
Update error messages
jlane9 Feb 18, 2021
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
81 changes: 46 additions & 35 deletions packages/@ionic/cli/src/commands/package/build.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ const IOS_ARTIFACT_TYPES = ['ipa', 'dsym'];
const ARTIFACT_TYPES = ANDROID_ARTIFACT_TYPES.concat(IOS_ARTIFACT_TYPES);
const TARGET_PLATFORM = ['Android', 'iOS - Xcode 11 (Preferred)', 'iOS - Xcode 10'];

interface Artifact {
name: string;
url: string;
artifact_type: string;
}

export interface PackageBuild {
job_id: number;
id: string;
Expand All @@ -48,6 +54,7 @@ export interface PackageBuild {
distribution_credential_name: string;
job: any;
distribution_trace: string;
artifacts: Artifact[];
}

interface DownloadUrl {
Expand Down Expand Up @@ -293,16 +300,6 @@ if you do not wish to download ${input('apk')}.
const token = await this.env.session.getUserToken();
const appflowId = await this.project.requireAppflowId();
const [ platform, buildType ] = inputs;
let artifactTypes;

if (Array.isArray(options['artifact-type'])) {
artifactTypes = options['artifact-type'].map(val => val.toUpperCase());
} else if (typeof options['artifact-type'] == 'string' && ARTIFACT_TYPES.includes(options['artifact-type'])) {
artifactTypes = [options['artifact-type'].toUpperCase()]
} else {
const useOriginalDefault = !!(options['build-file-name']);
artifactTypes = await this.getDefaultArtifactType(platform, useOriginalDefault);
}

if (!options.commit) {
options.commit = (await this.env.shell.output('git', ['rev-parse', 'HEAD'], { cwd: this.project.directory })).trim();
Expand All @@ -318,7 +315,7 @@ if you do not wish to download ${input('apk')}.
['Commit', strong(`${build.commit.sha.substring(0, 6)} ${build.commit.note}`)],
['Target Platform', strong(build.stack.friendly_name)],
['Build Type', strong(build.build_type)],
['Artifact Type(s)', strong(`${artifactTypes}`)],

@jlane9 jlane9 Feb 17, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I changed this and the above code, since AAB or DSYM may not always be available by default.

['Artifact Type(s)', options['artifact-type'] ? strong(`${options['artifact-type']}`.toUpperCase()) : weak('all available')],
['Security Profile', build.profile_tag ? strong(build.profile_tag) : weak('not set')],
['Environment', build.environment_name ? strong(build.environment_name) : weak('not set')],
['Native Config', build.native_config_name ? strong(build.native_config_name) : weak('not set')],
Expand All @@ -335,24 +332,28 @@ if you do not wish to download ${input('apk')}.
throw new Error(`Build ${build.state}`);
}

for (const artifactType of artifactTypes) {
const url = await this.getDownloadUrl(appflowId, buildId, artifactType, token);
const availableArtifactTypes = build.artifacts.map((artifact) => artifact.artifact_type);

if (!url.url) {
throw new Error('Missing URL in response');
if (Array.isArray(options['artifact-type'])) {
let errors = [];
for (const artifactType of new Set(options['artifact-type'])) {
try {
await this.downloadArtifact(appflowId, buildId, artifactType, token, options);
} catch (error) {
errors.push(error);
}
}

let customBuildFileName = '';

if (options['build-file-name']) {
this.env.log.warn(`The ${input('--build-file-name')} option has been deprecated. Please use ${input('--ipa-name')}, ${input('--apk-name')} or ${input('--{ artifact }-name')}.`);
customBuildFileName = await this.sanitizeString(options['build-file-name']);
} else {
customBuildFileName = await this.sanitizeString(options[`${artifactType.toLowerCase()}-name`]);
if (errors.length) {
throw new FatalException(`There were issues downloading artifacts: ${errors.join('\n')}`);
}

const filename = await this.downloadBuild(url.url, customBuildFileName);
this.env.log.ok(`Artifact downloaded: ${filename}`);
} else if (typeof options['artifact-type'] == 'string') {
await this.downloadArtifact(appflowId, buildId, options['artifact-type'], token, options);
} else {
for (const artifactType of availableArtifactTypes) {
await this.downloadArtifact(appflowId, buildId, artifactType.toUpperCase(), token, options);
}
}
}

Expand All @@ -369,17 +370,6 @@ if you do not wish to download ${input('apk')}.
return String(value);
}

async getDefaultArtifactType(platform: string, useOriginalDefault?: boolean): Promise<Array<string>> {

@jlane9 jlane9 Feb 17, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was deleted to instead download all available artifacts that the build has available to it.

switch (platform) {
case 'ios':
return useOriginalDefault ? ['IPA'] : ['IPA', 'DSYM'];
case 'android':
return useOriginalDefault ? ['APK'] : ['APK', 'AAB'] ;
default:
throw new Error(`No default artifact type for platform '${platform}'`);
}
}

async createPackageBuild(appflowId: string, token: string, platform: string, buildType: string, options: CommandLineOptions): Promise<PackageBuild> {
const { req } = await this.env.client.make('POST', `/apps/${appflowId}/packages/verbose_post`);
req.set('Authorization', `Bearer ${token}`).send({
Expand Down Expand Up @@ -502,4 +492,25 @@ if you do not wish to download ${input('apk')}.
fs.unlinkSync(tmpFile);
return filename;
}

async downloadArtifact(appflowId: string, buildId: number, artifactType: string, token: string, options: CommandLineOptions) {

const url = await this.getDownloadUrl(appflowId, buildId, artifactType.toUpperCase(), token);

if (!url.url) {
throw new Error(`Artifact type '${artifactType}' not found`);
}

let customBuildFileName = '';

if (options['build-file-name']) {
this.env.log.warn(`The ${input('--build-file-name')} option has been deprecated. Please use ${input('--ipa-name')}, ${input('--apk-name')} or ${input('--{ artifact }-name')}.`);
customBuildFileName = await this.sanitizeString(options['build-file-name']);
} else {
customBuildFileName = await this.sanitizeString(options[`${artifactType.toLowerCase()}-name`]);
}

const filename = await this.downloadBuild(url.url, customBuildFileName);
this.env.log.ok(`Artifact downloaded: ${filename}`);
}
}