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.

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

Feature/sc 124023/get refactor #704

Merged
merged 5 commits into from
Dec 21, 2023

Conversation

hugomontero
Copy link
Contributor

@hugomontero hugomontero commented Dec 18, 2023

Description

This PR contains all changes related with logic-function refactor for the get command.

Important!

Currently the logic.json creation doesn't have the right structure. That error were fixed in this PR #705

How to Test

  1. Pull down the branch: git pull && git checkout feature/sc-124023/get-refactor
  2. Install dependencies: npm i
  3. Run tests: npm run test:ci
  4. Attempt to:
    • List logic functions: npm start -- lf list --org ${my_org} || npm start -- lf list
    • Attempt to get one lf either for sandbox or org: npm start -- lf get --name "my logic function" --org ${my_org} /my/path/
    • Attempt to call get lf without params: npm start -- lf get

outcome

  • Test should pass
  • list should show your lf
  • on getting specific lf it will create two files in the path you pick
  • on calling without params (--name) you will be able to select from a list of your lf

Related Issues / Discussions

Story details: https://app.shortcut.com/particle/story/124023

Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA
  • Problem and solution clearly stated
  • Tests have been provided
  • Docs have been updated
  • CI is passing

@hugomontero hugomontero changed the base branch from feature/logic-function to feature/sc-124023/create-logic-function-helper-to-put-file December 18, 2023 20:30
// TODO - implement
throw new Error(`Not implemented yet for ${id} or ${name} or ${list}`);
// throw new Error(`Not implemented yet for ${id} or ${name}`);
const logicFunctionData = list.find(lf => lf.id === id || lf.name === name);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even thought this one just filter the list (can be handled directly from cmd) I decided keep it in case in the future we want to get this directly from the cloud instead of listing all logic functions

Comment on lines +116 to +140
function createAPIErrorResult({ error: e, message, json }){
const error = new VError(formatAPIErrorMessage(e), message);
error.asJSON = json;
return error;
}

function formatAPIErrorMessage(error){
error = normalizedApiError(error);

if (error.body){
if (typeof error.body.error === 'string'){
error.message = error.body.error;
} else if (Array.isArray(error.body.errors)){
if (error.body.errors.length === 1){
error.message = error.body.errors[0];
}
}
}

if (error.message.includes('That belongs to someone else.')){
error.canRequestTransfer = true;
}

return error;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be in api.js file maybe? I saw this repetitive code in every cmd file but it's more related with how we treat api errors.

@hugomontero hugomontero marked this pull request as ready for review December 21, 2023 14:57
Copy link
Member

@monkbroc monkbroc left a comment

Choose a reason for hiding this comment

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

Looks good!

@hugomontero hugomontero changed the base branch from feature/sc-124023/create-logic-function-helper-to-put-file to feature/logic-function December 21, 2023 17:05
@monkbroc monkbroc force-pushed the feature/sc-124023/get-refactor branch from ab88b25 to 4c248ff Compare December 21, 2023 17:05
@hugomontero hugomontero merged commit dd2dd19 into feature/logic-function Dec 21, 2023
1 of 4 checks passed
@monkbroc monkbroc deleted the feature/sc-124023/get-refactor branch January 19, 2024 19:40
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.

None yet

2 participants