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

Do not show invalid images as no_rights in costState #3902

Merged
merged 1 commit into from
Oct 24, 2022

Conversation

andrew-nowak
Copy link
Member

What does this change?

I did this in #3854 to try and override all other costs when an image is
missing credit or description (one definition of "valid"). Instead it
overrides all other costs whenever an image is uncroppable (restricted,
overquota, etc., the other definition of "valid").

costState is used to determine both which flags to show on the search
results page, and also the warnings/error messages on the image page,
which means that this change hid the restriction text from ALL
unpermissioned users! So revert this logic now, and commit to finding a
better way of applying the invalid flag in the future.

How should a reviewer test this change?

Look at an image with restrictions while without the edit_metadata permission. Can you see the restriction text? If you view the image in the search results page, you will unfortunately see only a yellow "restricted" flag, rather than a more descriptive red "!" flag.

I did this in #3854 to try and override all other costs when an image is
missing credit or description (one definition of "valid"). Instead it
overrides all other costs whenever an image is uncroppable (restricted,
overquota, etc., the other definition of "valid").

costState is used to determine both which flags to show on the search
results page, and also the warnings/error messages on the image page,
which means that this change hid the restriction text from ALL
unpermissioned users! So revert this logic now, and commit to finding a
better way of applying the invalid flag in the future.
@andrew-nowak andrew-nowak requested a review from a team as a code owner October 24, 2022 12:33
Copy link
Contributor

@paperboyo paperboyo left a comment

Choose a reason for hiding this comment

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

Code-blind approval (of functionality): 👍.

A card to make Allow lease teal not override invalid imagery (missing description and/or credit) here.

Copy link
Contributor

@phillipbarron phillipbarron left a comment

Choose a reason for hiding this comment

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

LGTM

@prout-bot
Copy link

Seen on auth, usage, collections, media-api, kahuna, image-loader, thrall, leases (merged by @andrew-nowak 8 minutes and 36 seconds ago) Please check your changes!

@prout-bot
Copy link

Seen on cropper (merged by @andrew-nowak 8 minutes and 44 seconds ago) Please check your changes!

@prout-bot
Copy link

Seen on metadata-editor (merged by @andrew-nowak 8 minutes and 48 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants