Feature/logic function get #696

keeramis · 2023-12-04T23:58:50Z

Description

This PR points to the execute command PR .

This PR does the following:

Implements the get command which downloads a logic function. The logic function can be specified using the --name or --id parameter on the CLI. Or it can be selected from the list of available logic functions
Uses list to get the name and id mapping to obtain one from the other
Improves color representation on the UI messages
Add a boolean parameter called display to the list method which controls whether to return the list for other helper methods or display it to the UI for the user
Add hyphen similar to create command if there are spaces (This will change as the specs change)
TODO: Remove the organization option in help (compare with the other PR)

How to Test

Do the setup drill (checkout the branch, install dependencies, etc)
Run this command npm start -- logic-function get and tinker with it using options --name, --id, --org

Usage includes but not limited to the following

npm start -- logic-function get
npm start -- logic-function get --name myLF
npm start -- logic-function get --id xxxx-xxxx-xxxx-xxxx-xxxx
npm start -- logic-function get --org Particle

Related Issues / Discussions

Completeness

src/cmd/logic-function.js

hugomontero · 2023-12-06T14:39:41Z

src/cmd/logic-function.js

-		// 3. Download it to files. Take care of formatting
+		const api = createAPI();
+		({ name, id } = await this._getLogicFunctionIdAndName(org, name, id));
+		const slugName = name.toLowerCase().replace(/\s/g, '-');


let's change it with slugify from utils

hugomontero · 2023-12-06T14:49:35Z

src/cmd/logic-function.test.js

+
+			await logicFunctionCommands.get({ name: 'LF1' });
+
+			expect(fs.existsSync(path.join(process.cwd(), 'LF1', 'LF1.js'))).to.be.true;


Pretty sure that's making linux tests fail.
I'd rather use fixtures/__root__/... folder for creation. Let's try to repro the test error in a linux machine.
the only thing that came to my mind now is lf1 dir and files should be in lower case since we're converting the name with slugify function.

ah that's a good point - that these might be in lower case!

hugomontero · 2023-12-06T15:45:27Z

src/cmd/logic-function.test.js

+	});
+
+	describe('get', async () => {
+		beforeEach(() => {


use afterEach to return everything to normal after your test

src/cmd/logic-function.js

hugomontero

I'd say let's merge it since we agree to change the dir creation in other story as well the list function

monkbroc

Keerthy and I reviewed this PR over Zoom. Keerthy has some notes on things to improve.

keeramis · 2023-12-08T01:06:08Z

@hugomontero @monkbroc I made the improvements we discussed yday.

Hugo, I modified the helpers a bit, but didn't make many changes to the create and execute commands because I know that you are working on them separately and I didn't want to interfere with your changes.

Please let me know if I am missing something. Hope the functionality is clearer now when reading the code.

keeramis · 2023-12-08T01:09:13Z

src/cmd/logic-function.js

+			choices : list,
+			nonInteractiveError: 'Provide name for the logic function'
+		});
+		return answer.logic_function;


@monkbroc I know we discussed to return name and id here. I didn't make that change to keep the _getLogicFunctionIdAndName look consistent in the way it calls the _getXFromY methods - Kindly let me know if this should have been different.

lf1/lf1.js

src/cmd/logic-function.js

monkbroc

Retest the get and create functions after the latest changes and once they work, go ahead and merge

monkbroc · 2023-12-08T16:54:09Z

src/cmd/logic-function.js

+	_printListHelperOutput() {
+		this.ui.stdout.write(`No Logic Functions currently deployed in your ${getOrgName(this.org)}.${os.EOL}`);
+		this.ui.stdout.write(`${os.EOL}`);
+		this.ui.stdout.write('To create a Logic Function, see  ' + this.ui.chalk.yellow('\'particle logic-function create\'.') + `${os.EOL}`);


I suggest you don't include the quote for commands inside of chalk.yellow. It makes it harder to copy paste. Also make sure the period at the end of the line is not in yellow.

Suggested change

this.ui.stdout.write('To create a Logic Function, see ' + this.ui.chalk.yellow('\'particle logic-function create\'.') + `${os.EOL}`);

this.ui.stdout.write(`To create a Logic Function, see ${this.ui.chalk.yellow('particle logic-function create')}.${os.EOL}`);

monkbroc · 2023-12-08T16:54:23Z

src/cmd/logic-function.js

+			this.ui.stdout.write(`	- ID: ${item.id}${os.EOL}`);
+			this.ui.stdout.write(`	- ${item.logic_triggers[0].type} based trigger ${os.EOL}`);
+		});
+		this.ui.stdout.write(`${os.EOL}To view a Logic Function's code, see \`particle logic-function get.\`${os.EOL}`);


Format the command in yellow

monkbroc · 2023-12-08T16:55:25Z

src/cmd/logic-function.js

+
+		const { dirPath, jsonPath, jsPath } = await this._generateFiles({ logicFunctionConfigData, logicFunctionCode, name });
+
+		this._printGetFilesOutput({ dirPath, jsonPath, jsPath });


I get this._printGetFilesOutput is not a function when I call particle logic-function get

monkbroc · 2023-12-08T16:57:04Z

src/cmd/logic-function.js

-			existingLogicFunction = existingLogicFunctions.find((item) => item.name === name);
-		} catch (error) {
+		const logicFuncNameExists = this.logicFuncList.find((item) => item.name === name);
+		if (!logicFuncNameExists) {
 			this.ui.stdout.write(this.ui.chalk.yellow(`Warn: We were unable to check if a Logic Function with name ${name} already exists.${os.EOL}`));


I get this message when I create a LF

? What would you like to call your Function? local test ? Please provide a short description of your Function: Creating Logic Function local test for Sandbox... Warn: We were unable to check if a Logic Function with name local test already exists. Successfully created local test in . Files created: - local-test/@types/particle_core.d.ts - local-test/@types/particle_encoding.d.ts - local-test/local-test.js - local-test/local-test.logic.json Guidelines for creating your Logic Function can be found <TBD>. Once you have written your Logic Function, run - 'particle logic-function execute' to run your Function - 'particle logic-function deploy' to deploy your new changes

monkbroc · 2023-12-08T16:58:08Z

src/cmd/logic-function.js

+	}
+
+	_printGetOutput({ dirPath, jsonPath, jsPath }) {
+		if (dirPath || jsonPath || jsPath) {


What is this if for?

monkbroc · 2023-12-08T17:00:09Z

src/cmd/logic-function.js

+	}
+
+	_printListOutput({ logicFunctionsList }) {
+		this.ui.stdout.write(`Logic Functions currently deployed in your ${getOrgName(this.org)}:${os.EOL}`);


General comment for another PR: make getOrgName return "your Sandbox" or the name of the org so that the messages print like this:
Logic Functions currently deployed in your Sandbox
Logic Functions currently deployed in Particle

instead of currently which would show
Logic Functions currently deployed in your Particle

keeramis force-pushed the feature/logic-function-get branch from d0f20e4 to af7e3d4 Compare December 5, 2023 00:00

hugomontero reviewed Dec 5, 2023

View reviewed changes

src/cmd/logic-function.js Outdated Show resolved Hide resolved

hugomontero force-pushed the feature/sc-123396/implement-execute-command-with-eventdata branch from fa201f4 to 1312a53 Compare December 5, 2023 16:15

Base automatically changed from feature/sc-123396/implement-execute-command-with-eventdata to feature/logic-function December 5, 2023 17:12

keeramis force-pushed the feature/logic-function-get branch from 76600b8 to b67f13d Compare December 5, 2023 18:03

hugomontero reviewed Dec 6, 2023

View reviewed changes

keeramis added 22 commits December 6, 2023 08:07

Return the list of logic functions

8edbee0

Implement Get command

c0aa78a

Reimplement Get command

fc36d11

Cleanup

fe16b6b

Remove older logic_function file

3b75797

Skeleton for tests

635f6a8

Add tests

5055999

Use existing list method instead of api call

2527214

Consolidate prompt functionality

fee7b45

Implemeted delete command

712e096

WIP

3e99183

rename methods

271ecbd

Fix tests

daf7589

Modify tests

8f28e1f

Fix prompt tests

8e30026

Add skeleton for tests

84573c4

Add slugName until we finalize the approach

b3b4f7b

[tests] _promptForLogicFunctionName

89b5267

[tests] _validateLFName, _selectLogicFunction

6fc4328

[test] _getLogicFunctionIdAndName

1a27b47

[tests] _validateDir

f6ec5bf

Add .logic.json

42c1792

keeramis marked this pull request as ready for review December 6, 2023 16:36

keeramis requested a review from monkbroc December 6, 2023 16:38

remove double negation for prompt overwrite

c4c9607

hugomontero reviewed Dec 6, 2023

View reviewed changes

src/cmd/logic-function.js Outdated Show resolved Hide resolved

Add a FIXME

c787fe5

keeramis requested a review from hugomontero December 6, 2023 18:07

remove line about --org

b7e1ea6

hugomontero reviewed Dec 6, 2023

View reviewed changes

src/cmd/logic-function.js Outdated Show resolved Hide resolved

hugomontero approved these changes Dec 6, 2023

View reviewed changes

monkbroc reviewed Dec 7, 2023

View reviewed changes

keeramis added 4 commits December 7, 2023 16:54

Address review comments

89671f6

Minor changes to create command

23f0bde

Minor changes to create command

e3b01bc

minor cleanup

97d3748

Lint fix

9791afe

keeramis commented Dec 8, 2023

View reviewed changes

keeramis requested review from monkbroc and hugomontero December 8, 2023 02:16

Reword the titles for tests

90f96a3

hugomontero reviewed Dec 8, 2023

View reviewed changes

lf1/lf1.js Outdated Show resolved Hide resolved

hugomontero reviewed Dec 8, 2023

View reviewed changes

src/cmd/logic-function.js Outdated Show resolved Hide resolved

hugomontero reviewed Dec 8, 2023

View reviewed changes

src/cmd/logic-function.js Outdated Show resolved Hide resolved

Remove dir after test

d3b185b

monkbroc approved these changes Dec 8, 2023

View reviewed changes

keeramis added 3 commits December 8, 2023 10:50

Address review comments

4df8817

Remove warning if name is not obtained as list query takes care of it

8c547a3

Rework getOrgName

bf60665

keeramis merged commit 6ec5118 into feature/logic-function Dec 8, 2023
5 checks passed

monkbroc deleted the feature/logic-function-get branch January 19, 2024 19:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/logic function get #696

Feature/logic function get #696

keeramis commented Dec 4, 2023 •

edited

Loading

hugomontero Dec 6, 2023

hugomontero Dec 6, 2023

keeramis Dec 6, 2023

hugomontero Dec 6, 2023

hugomontero left a comment

monkbroc left a comment

keeramis commented Dec 8, 2023

keeramis Dec 8, 2023

monkbroc left a comment

monkbroc Dec 8, 2023

monkbroc Dec 8, 2023

monkbroc Dec 8, 2023

monkbroc Dec 8, 2023

monkbroc Dec 8, 2023

monkbroc Dec 8, 2023


		await logicFunctionCommands.get({ name: 'LF1' });

		expect(fs.existsSync(path.join(process.cwd(), 'LF1', 'LF1.js'))).to.be.true;

	this.ui.stdout.write('To create a Logic Function, see ' + this.ui.chalk.yellow('\'particle logic-function create\'.') + `${os.EOL}`);
	this.ui.stdout.write(`To create a Logic Function, see ${this.ui.chalk.yellow('particle logic-function create')}.${os.EOL}`);


		const { dirPath, jsonPath, jsPath } = await this._generateFiles({ logicFunctionConfigData, logicFunctionCode, name });

		this._printGetFilesOutput({ dirPath, jsonPath, jsPath });

Feature/logic function get #696

Feature/logic function get #696

Conversation

keeramis commented Dec 4, 2023 • edited Loading

Description

How to Test

Related Issues / Discussions

Completeness

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hugomontero left a comment

Choose a reason for hiding this comment

monkbroc left a comment

Choose a reason for hiding this comment

keeramis commented Dec 8, 2023

Choose a reason for hiding this comment

monkbroc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keeramis commented Dec 4, 2023 •

edited

Loading