talis-fb / TReq

A CLI tool for effortless HTTP requests
GNU General Public License v3.0
55 stars 2 forks source link

Command for rename saved requests #12

Closed talis-fb closed 8 months ago

talis-fb commented 9 months ago

Implement a new command that allows users to rename saved requests from the system.

treq rename my-request new-name

You can imagine this command as the same thing of this below, without doing request...

treq run my-request --save-as new-name

Expected Behavior:

SummerGram commented 8 months ago

Hi @talis-fb,

I will work on this issue. I am thinking to follow these steps.

  1. Check if "new-name" file is existed.
  2. If not existed, the program will copy the content in the "my-request" file to a new file "new-name".
  3. Otherwise, it will prompt the message to indicate the user to manually delete the "new-name".

I have 1 scenario. Suppose I have 2 programs. One is renaming the my-request while another one is running the my-request.

Will it be conflict?

talis-fb commented 8 months ago

Awesome. @SummerGram !! The steps are all right, but is also need to add one final step: removing the "my-request" data once it has been successfully copied to "new-name". I missed this detail in Issue description. But it's all right.

About the scenario, there's no need to worry about it, as the CLI app is not intended to be executed in this manner. Users are only expected to run TReq sequentially and one at a time.

SummerGram commented 8 months ago

Good! @talis-fb

Thanks for the information.

The procedure should be like this.

  1. Check if the "my-request" exists.
  2. If not, prompt the error message. Otherwise, proceed to step 3.
  3. Check if the "new-name" exists.
  4. If yes, prompt the error message. Otherwise, proceed to step 4.
  5. Run "std::fs::rename"

I will create two functions in CommandsFactory.

Screenshot 2024-02-17 at 11 53 47 PM
talis-fb commented 8 months ago

Exactly! @SummerGram

SummerGram commented 8 months ago

@talis-fb Actually, I think we can create function in utils/file.rs.

Screenshot 2024-02-18 at 12 31 45 AM

We just find out the PathBuf. Then pass to the function. If any error, we can output the errors.

talis-fb commented 8 months ago

That's work, and it's a great ideia, but in current state of App architecture is better do this...

The "FileService" is meant to create/get/remove/rename files in only "context folder aplication". What I mean is that the File Service should only do those operation in files inside /tmp, $HOME/.config/treq and $HOME/.local/share/treq. Only inside this three folders. That's why the method in Facade only receive Strings, instead a PathBuf (Instead the "remove_file" but it's something to be fixex) : https://github.com/talis-fb/TReq/blob/develop/src/app/services/files/facade.rs

What you actually should do specifically is create this two method you send here in "facade", using only sync abstration, like std::fs. You can follow inspiration from other method of facade: https://github.com/talis-fb/TReq/blob/develop/src/app/services/files/service.rs.

Then, in this file you create a command and use the two methods you defined in facade to be called here.

from service.rs and facade.rs

fn check_exist_data_file(&self, path: String) -> Result<bool>;
fn rename_data_file(&self, path: String, new_name: String) -> Result<()>;

from commands/requests.rs

pub fn rename_file_saved_request(request_name: String, new_name: String) -> CommandFileService<Result<()>> {
talis-fb commented 8 months ago

@SummerGram Did you understand? Any question you can ask

SummerGram commented 8 months ago

@talis-fb

So you would like to separate the backend and group all related operation into one. The utils/files.rs is not for the backend.rs.

SummerGram commented 8 months ago

@talis-fb

They are the successful and failed message respectively.

Screenshot 2024-02-18 at 1 19 14 AM Screenshot 2024-02-18 at 1 21 50 AM

I will add the --no-confirm later. Probably need to validate the number of input variables later.

talis-fb commented 8 months ago

Awesome!

talis-fb commented 8 months ago

@talis-fb

So you would like to separate the backend and group all related operation into one. The utils/files.rs is not for the backend.rs.

@SummerGram actually, the utils/files.rs is indeed to be used exactly in backend.rs. Like HERE.

The separation is:

As you can check, all the functions in utils/files.rs are used only (and just only) in backend.rs. To parse, deserialize, or editing the content of files. The FileService only call sync function of file manipulation in implementation of service facade.rs

SummerGram commented 8 months ago

@talis-fb

Suppose the user type without --no-confirm. How do the program handle the case?

I suppose it asks for permission in the function parse_inputs_to_main_command_choices in main_command_choices.rs.

talis-fb commented 8 months ago

Actually no. The confirmation to use is made in the command executor you implement for Rename command. With "command executor" i mean like THIS. This is the command executor of inspect. When user run treq inspect ... this is the final main function will happen. After all the parser, the map_input_to_commands and the get_executor this struct will be used to run this execute() function. All detail of the command is implement in this function. In you case, this behavior of asking user for confirmation is a internal logic of rename command. A detail of it. Then, you'll need to create a RenameExecutor and put this "confirmation" in this function (before all main actions).

talis-fb commented 8 months ago

Beyond this, just to point out another suggestion you may ask on how you can implement this "no-confirm"...

  1. In THIS CliCommandChoice add the "no-confirm" field as a boolean option.
  2. Doing this also in HERE
  3. Pass this option HERE too. Inside this Executor is where the "switch" of --no-confirm should live, like this confirmation I said in last comment. It could be something inside a if !no-confirm.
talis-fb commented 8 months ago

@SummerGram