feat(archive): revamp archive-upload view

[tthvo]

Fixes #510
Fixes #521 (dup)
Fixes #522

Feat:

• Users can now add/upload labels for any uploaded archived recordings via label editor and upload button.

Fix:

• Label bulk-edit should work in upload archives view (i.e. corrected api calls and added null check).

Chore:

• Clean up RecordingLabelField component to reduce re-rendering.
• Rename custom test render for consistency.
• Clean up ArchivedRecordingTable to use props.target for consistency.

[andrewazores]

Works really nicely. I think the nested modal dialogs for uploading a recording and then "uploading" the associated metadata feels a little funny, though. It is kinda nice that this allows the user to upload/submit multiple metadata files for a single recording so they can add a lot of labels this way, I suppose, but I imagined this working more as either having a single file chooser which would ask the user just to select a JFR file to upload and the associated labels JSON would be read and applied automatically if it also existed, otherwise if that's not feasible then the modal would have two separate file choosers for the recording and for the metadata, and both would be submitted at once.

[lkonno]

I am unsure if it should be covered by this PR or I should open a new issue, but it seems that the Edit Labels button for the uploads is not working.

[andrewazores]

Good catch, I didn't try/notice that. It should be part of this PR, I think, unless that bug is also present in other places where Edit Labels is present.

[tthvo]

Good catch, I didn't try/notice that. It should be part of this PR, I think, unless that bug is also present in other places where Edit Labels is present.

Oh yess, noticed this happens only in the uploaded archive views. Still happens before refactoring so I am looking into this :D

[tthvo]

Works really nicely. I think the nested modal dialogs for uploading a recording and then "uploading" the associated metadata feels a little funny, though. It is kinda nice that this allows the user to upload/submit multiple metadata files for a single recording so they can add a lot of labels this way, I suppose, but I imagined this working more as either having a single file chooser which would ask the user just to select a JFR file to upload and the associated labels JSON would be read and applied automatically if it also existed, otherwise if that's not feasible then the modal would have two separate file choosers for the recording and for the metadata, and both would be submitted at once.

Right! I probably add two inputs, (not sure how to read more files than prompted by the browser). Do we still need a label editor for adding new labels here too?

I am thinking of another way is that when the user clicks on the upload button (inside label editor), they are directly prompted to select a file. Once selected, the file is read and label editors are filled with those labels if any. This way we can skip the metadata modal. What do you think?

[andrewazores]

Sure, that sounds good too. This way the user can select the file and have its contents automatically added to the labels visible on the upload modal. If they do this multiple times then the latest file read should merge labels, overwriting anything that was there previously if there are any label key collisions. I think it would still be good to have the label editor controls present on this one modal too, so the user can fine-tune their labels manually before uploading the recording.

[tthvo]

Sure, that sounds good too. This way the user can select the file and have its contents automatically added to the labels visible on the upload modal. If they do this multiple times then the latest file read should merge labels, overwriting anything that was there previously if there are any label key collisions. I think it would still be good to have the label editor controls present on this one modal too, so the user can fine-tune their labels manually before uploading the recording.

Right! Also do you think it's a better option to not overwrite any existing keys but shows an error helper-text under any duplicate keys? This way, the user can choose which one to remove?

[andrewazores]

That's a good option too. If there are any conflicts then the helper text should be red, or maybe the label itself should have a red warning outline, and the submission button should be disabled.

[tthvo]

That's a good option too. If there are any conflicts then the helper text should be red, or maybe the label itself should have a red warning outline, and the submission button should be disabled.

Right! I added it this such that individual key/value cells are indicated with warning if invalid. Also, implemented the new flow for uploading recording metadata. Ready for review :D

[andrewazores]

If the labels are invalid because of non-unique keys then I'm still able to Submit on the modal. The upload succeeds and I think the last-uploaded/last-added labels are used?

In this case I think I should be blocked from submitting the modal.

Also perhaps the button should be renamed to the plural "Upload Labels" since the JSON file can and often will contain multiple?

[tthvo]

If the labels are invalid because of non-unique keys then I'm still able to Submit on the modal. The upload succeeds and I think the last-uploaded/last-added labels are used?

In this case I think I should be blocked from submitting the modal.

Also perhaps the button should be renamed to the plural "Upload Labels" since the JSON file can and often will contain multiple?

Oh right I thought i fixed that. Let me have a closer look. And I will rename the button too.

Editted: Oh I think selecting a valid recording file overwrites invalidity of labels. Will fix this now.

[maxcao13]

One thing about the UI itself, when I click the 'Show metadata options', as a user it feels annoying that the button goes up after clicking on the dropdown and then you have to move your mouse up to click it again because of the modal resizing, but it just might be me. Thoughts?

Other than that looks great!

[maxcao13]

Also, I'm not sure if I would know what the naming convention for cryostat archived recordings as a user, would it make sense to have a helper icon somewhere to mention this?

[tthvo]

Also, I'm not sure if I would know what the naming convention for cryostat archived recordings as a user, would it make sense to have a helper icon somewhere to mention this?

Oh yess been thinking about that. I will add this.

[tthvo]

Something like this for naming convention?

[maxcao13]

Something like this for naming convention?
screenshot

I think it should be target-name_recordingName_timestamp.jfr, @andrewazores thoughts?

EDIT: Also maybe is it possible to have the ? in-line as well so the users will know to hover over it?
i.e. Select a JDK Flight .... and follow the naming convention^[?] used by Cryostat...

[tthvo]

One thing about the UI itself, when I click the 'Show metadata options', as a user it feels annoying that the button goes up after clicking on the dropdown and then you have to move your mouse up to click it again because of the modal resizing, but it just might be me. Thoughts?

Other than that looks great!

Yehh agreed but the way the modal resizing is to make sure itself it is centered, so there is not much we can do about it :(

[maxcao13]

One thing about the UI itself, when I click the 'Show metadata options', as a user it feels annoying that the button goes up after clicking on the dropdown and then you have to move your mouse up to click it again because of the modal resizing, but it just might be me. Thoughts?
Other than that looks great!

Yehh agreed but the way the modal resizing is to make sure itself it is centered, so there is not much we can do about it :(

No worries then!

[tthvo]

Okayy, not really a common way to do it in Patternfly but with some hacks. What do you think? Added the superscript at the end for cleaner layout.

[maxcao13]

Okayy, not really a common way to do it in Patternfly but with some hacks. What do you think? Added the superscript at the end for cleaner layout.
screenshot

Looks nice! I would put the title as Archive naming conventions: and have a new line after that but I'm no way a UI designer so I would say wait for further input.

[andrewazores]

You could change the example to look a little more generic? Either just something very placeholder like com-example-myapp_profiling_timestamp.jfr or use Cryostat itself like io-cryostat-Cryostat_profiling_timestamp.jfr.

[andrewazores]

^ looks good otherwise

[tthvo]

Thanks a lot for the inputs. Here is the new look. I tried adding a bit more spacing between 2 sentence but it looked very oddly far away so the current one is probably the best option now.