Skip to content

Conversation

@apata
Copy link
Contributor

@apata apata commented Dec 4, 2025

Changes

  • To make internal stats API requests for password-protected shared links, shared link auth cookie must be set in the requests.

Tests

  • Automated tests have been added

Changelog

  • Entry has been added to changelog

Documentation

  • This change does not need a documentation update

Dark mode

  • This PR does not change the UI

@apata apata requested a review from a team December 4, 2025 09:37
@cnkk cnkk added the preview label Dec 4, 2025
@github-actions
Copy link

github-actions bot commented Dec 4, 2025

Preview environment👷🏼‍♀️🏗️
PR-5932

Copy link
Contributor

@ukutaht ukutaht left a comment

Choose a reason for hiding this comment

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

One comment about naming, otherwise LGTM 👍

end

def get_type(%__MODULE__{password_hash: hash}) when not is_nil(hash), do: :password_protected
def get_type(%__MODULE__{}), do: :unlisted
Copy link
Contributor

Choose a reason for hiding this comment

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

naming: :unlisted doesn't feel descriptive to me, not sure if it's some kind of terminology I'm not familiar with? How about :public?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I agree that it's a bit mystical, but that name was in the codebase already. I'm wary of :public because it might be confused with the fully public dashboard.

I can go simple with :with_password, :without_password or just def password_protected?(link) true / false? If we go with the boolean, matching on it isn't as pretty in with ... <- {function name}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In plugins API, shared_link schema has a field :password_protected. I've decided to unify the checks with that in b8bb107

@apata apata requested a review from ukutaht December 5, 2025 09:25
@apata apata added this pull request to the merge queue Dec 8, 2025
Merged via the queue into master with commit 007155b Dec 8, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants