Skip to content
This repository has been archived by the owner on Oct 5, 2023. It is now read-only.

feature: adding possibility to remove file #439

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

soleksy-splunk
Copy link
Contributor

passing temp name during edit
for files that were already submited before

name passed only for edit action for rest is null
BaseFormView > Control Wrapper > FileInputComponent

@soleksy-splunk
Copy link
Contributor Author

How it looks right now:
File name is not returned from backend so I just put "Previous File" just to put anything there

Edit page
Screenshot 2023-09-27 at 21 16 09
w:

Clone page:
Screenshot 2023-09-27 at 21 21 13

file possibilities:

  1. file is encrypted and required:
  • we got only the possibility to remove the current one and add a new one, it might be confusing as when encrypted the File name is displayed but value i empty so at update push we see this error
    value is empty just temp file name
Screenshot 2023-09-27 at 21 48 03
  1. file is not encrypted but not required
  • we got all possibilities, value is used correctly (after edit/clone, value is correct) also got the possibility to delete complitely the file
  1. file is not encrypted but required
    used same as 2, just after deletion we must push another file

  2. file is encrypted and but not required:
    - previouse value of file is set as empty but we push File name "Previous File", which might be misleading, if user just updates other files and pushes changes without pushing new File it will be pushed incorrectly as encoded code

@artemrys as you raised ticket please have a look at those problems:

  1. in 1st point it might be confusing but user can't break data
  2. in 4rd point user might be able to brake data, currently the situation is the same but maybe it will be good to highlight it somehow?

@artemrys
Copy link
Member

artemrys commented Oct 2, 2023

File name is not returned from backend so I just put "Previous File" just to put anything there

You are right, there is no concept of a "file" in the backend. We just save the content of the file if we can read it.

in 1st point it might be confusing but user can't break data

If "file" field is encrypted and required then we must ask user to upload a file every time they want to make a change to that entity. It is not convenient, I do agree, but I do not see how we can make it better.

in 4rd point user might be able to brake data, currently the situation is the same but maybe it will be good to highlight it somehow?

If a user previously uploaded a file and then wants to empty that value then we are sending empty string as a content? But the backend is still going to attempt to encrypt it. Which results in saving some encrypted empty string which add-on developer need to process themselves.

@soleksy-splunk
Copy link
Contributor Author

soleksy-splunk commented Oct 3, 2023

@artemrys
in 4rd point the problem appears when user does not update file at all, those are the points that couse problem:

  1. due to encrypted file data the base value is empty string,
  2. user sees there "Previous File" as file name, so someone might suspecting that this file is correct etc.
  3. due to file is not required there is a possibility to push that empty value

so to sum up the confusion might be that user thinks he is still using the old file but due to it is encrypted the value there is empty string

idea: maybe when we got both of them provided we can just display a simple label "File is empty"/"Value of the file is empty" or sth like that.

currently that issue is still there, so thats not something we are creating, but maybe we can improve it somehow

passing temp name during edit
for files that were already submited before

name passed only for edit action for rest is null
BaseFormView > Control Wrapper > FileInputComponent
adding remove file possibility while doing cloning process
@soleksy-splunk soleksy-splunk force-pushed the ADDON-65000---UCC]-File-component-should-support-deletion branch from e736338 to 597bf67 Compare October 3, 2023 09:22
@artemrys
Copy link
Member

artemrys commented Oct 3, 2023

@artemrys in 4rd point the problem appears when user does not update file at all, those are the points that couse problem:

  1. due to encrypted file data the base value is empty string,
  2. user sees there "Previous File" as file name, so someone might suspecting that this file is correct etc.
  3. due to file is not required there is a possibility to push that empty value

so to sum up the confusion might be that user thinks he is still using the old file but due to it is encrypted the value there is empty string

idea: maybe when we got both of them provided we can just display a simple label "File is empty"/"Value of the file is empty" or sth like that.

currently that issue is still there, so thats not something we are creating, but maybe we can improve it somehow

Do you think we can solve this issue by simply not allowing a file type to be encrypted and not required?

@soleksy-splunk
Copy link
Contributor Author

soleksy-splunk commented Oct 3, 2023

Do you think we can solve this issue by simply not allowing a file type to be encrypted and not required?

@artemrys
yea that should solve the problem, but when i think generally about files I can imagine a scenario when file is optional but also i would like it to be encrypted, not standard scenario but might happen.

Other possibility i see is journey were only if file is encrypted and optional plus user tries to push that old encrypted file without any change, some warning pops up "You can't use previous file", so you basically need to delete it manually if you do not want that change to happen at all, then the user is aware that empty file will be pushed or at least it is more intuitive.

Something else might be that for this case we can just remove that filename, so it looks more like there is not any file for this at all.

when we display file name and data for this file are encrypted
then we need to display error message that file needs to be reuploaded
@artemrys
Copy link
Member

artemrys commented Oct 4, 2023

The UI repo was migrated, can you please open another PR for UCC generator repository?

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