Skip to content
This repository was archived by the owner on Apr 15, 2024. It is now read-only.

[FAB-37] Campaign Import Export commands - #279

Merged
jgielstra-cs merged 10 commits into
developfrom
FAB-37
Dec 14, 2020
Merged

[FAB-37] Campaign Import Export commands#279
jgielstra-cs merged 10 commits into
developfrom
FAB-37

Conversation

@ljha-CS

@ljha-CS ljha-CS commented Dec 14, 2020

Copy link
Copy Markdown
Collaborator

Merged AMP-Commands branch also, as these commands should go first

Checklist:

Please check you fulfill ALL of the relevant checkboxes

  • Notified docs of any potential USER-facing changes
  • Added short description of the change - with relevant motivation and context.
  • Branch has the ticket number in its name (along with a ticket summary)
  • Commented the code, particularly in hard-to-understand areas
  • Added tests that prove my fix is effective or that my feature works
  • Ran npm test and it passes
  • Changes generate no new warnings

@ljha-CS
ljha-CS requested a review from jgielstra-cs December 14, 2020 15:07
Comment thread package.json
"@hapi/joi": "17.1.1",
"boxen": "4.2.0",
"chalk": "4.0.0",
"form-data": "3.0.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why multi-part ??

Comment thread src/client/catalog.js
});
}

importCampaign(projectId, token, filepath, deploy, overwrite) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We typically just POST file data ?? Why are we using multipart here ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

With streaming file, import API was throwing Unsupported Content Type error. This API expect form data. cURL examples https://docs.google.com/document/d/1RJwLVsFeYpHu1sDfhztLnnEcJ6iIu_yDFfjZmEu_2cc/edit

Comment thread src/commands/missions.js Outdated
printSuccess, printError, filterObject, parseObject, printTable,
} = require('./utils');

module.exports.CreateMissionCommand = class CreateMissionCommand {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Need to confirm we are supporting import/export of missions ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

AMP Commands branch (from which this branch is created) is old (last updated 10th Nov). Should I remove mission related code from this PR?

@ghost
ghost requested a review from lkrishna-cs December 14, 2020 16:57
@ghost
ghost requested a review from mcriscolo-cs December 14, 2020 18:51
@jgielstra-cs
jgielstra-cs merged commit be2d0f6 into develop Dec 14, 2020
@jgielstra-cs
jgielstra-cs deleted the FAB-37 branch December 14, 2020 19:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants