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

Connected object tags API #421

Merged
merged 1 commit into from
Nov 20, 2020
Merged

Connected object tags API #421

merged 1 commit into from
Nov 20, 2020

Conversation

bexsoft
Copy link
Collaborator

@bexsoft bexsoft commented Nov 19, 2020

What does this do?

Connects object tags API to object details screen.

NOTE: There is a small delay between tag changes after saving any modification, will fix this after adding loaders to object details page.

How does it look?

Screen Shot 2020-11-19 at 13 16 26

Screen Shot 2020-11-19 at 14 58 20

Screen Shot 2020-11-19 at 13 16 07

Screen Shot 2020-11-19 at 13 15 58

@bexsoft bexsoft added the UI User Interface label Nov 19, 2020
@bexsoft bexsoft added this to the V1 Drop milestone Nov 19, 2020
@bexsoft bexsoft self-assigned this Nov 19, 2020
@bexsoft bexsoft added this to Review in progress in Console Version 1 via automation Nov 19, 2020
dvaldivia
dvaldivia previously approved these changes Nov 19, 2020
Copy link
Collaborator

@cesnietor cesnietor left a comment

Choose a reason for hiding this comment

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

I saw that we set the same value on key and value but minio allows them to be different, so if the object has tags already set by mc we would show only one value:

mc tag list s3/testbucket/testobject
Name                :    testobject
editable            :    only-by-owner-and-authenticated
confidentiality     :    open-to-authenticated-only

we might need to change to show/set both key and values.

@dvaldivia
Copy link
Collaborator

@cesnietor would showing that need some changes to the UI?

@cesnietor
Copy link
Collaborator

@cesnietor would showing that need some changes to the UI?

yes probably a redesign or do a key:value on tag itself.

@bexsoft
Copy link
Collaborator Author

bexsoft commented Nov 19, 2020

@cesnietor @dvaldivia But in this case what would be the purpose of showing the key too?, wouldn't it be better to make the labels as string arrays? I think people would be used to look for complete phrases rather than key values

@cesnietor
Copy link
Collaborator

But in this case what would be the purpose of showing the key too?, wouldn't it be better to make the labels as string arrays? I think people would be used to look for complete phrases rather than key values

yes, I agree, but since we mirror mc's behavior we need to do an equivalent thing. Also it is an AWS thing https://docs.aws.amazon.com/general/latest/gr/aws_tagging.html

@bexsoft bexsoft added the WIP This PR is WIP and cannot be merged yet label Nov 19, 2020
@bexsoft
Copy link
Collaborator Author

bexsoft commented Nov 20, 2020

I saw that we set the same value on key and value but minio allows them to be different, so if the object has tags already set by mc we would show only one value:

mc tag list s3/testbucket/testobject
Name                :    testobject
editable            :    only-by-owner-and-authenticated
confidentiality     :    open-to-authenticated-only

we might need to change to show/set both key and values.

Added changed to be S3 Compliant:

Screen Shot 2020-11-19 at 19 00 11

Screen Shot 2020-11-19 at 19 00 03

Screen Shot 2020-11-19 at 18 59 56

Screen Shot 2020-11-19 at 18 59 43

Copy link
Collaborator

@cesnietor cesnietor left a comment

Choose a reason for hiding this comment

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

tested LGTM

Console Version 1 automation moved this from Review in progress to Reviewer approved Nov 20, 2020
@dvaldivia dvaldivia merged commit 8a6a75b into minio:master Nov 20, 2020
Console Version 1 automation moved this from Reviewer approved to Done Nov 20, 2020
@bexsoft bexsoft deleted the connect-tags branch November 25, 2020 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI User Interface WIP This PR is WIP and cannot be merged yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants