Skip to content

Make human readable short urls #92

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

Merged
merged 10 commits into from
Jul 11, 2022
Merged

Make human readable short urls #92

merged 10 commits into from
Jul 11, 2022

Conversation

bbucsy
Copy link
Member

@bbucsy bbucsy commented Jul 3, 2022

So this pr solves issue with the monkey patching private albums
and greatly shortens the path to images without introducing another redirect, so
it is easier for browsers to cache 🥳

It uses friendly_id gem to make human friendly urls from filenames.

The good part is we don't have to replace old links on the blog, because they will still work.

If PR is accepted please ping me, because i want to deploy it to lois. There is a data migration rake task, to move old images to the new AlbumImage model, but if something goes bad i want to be there to put out the dumpster fire 🔥

closes #91 and closes #90

@bbucsy bbucsy changed the title Feat make human readable short urls Make human readable short urls Jul 3, 2022
@bbucsy bbucsy requested review from triszt4n and SepsiLaszlo July 3, 2022 19:20
@bbucsy bbucsy marked this pull request as ready for review July 3, 2022 19:29
Copy link
Contributor

@SepsiLaszlo SepsiLaszlo left a comment

Choose a reason for hiding this comment

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

Great work, really spicy. 🌶️
Much appreciated! 🎆

Copy link
Member

@triszt4n triszt4n left a comment

Choose a reason for hiding this comment

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

Somehow I need to log in to reach these images with the shorter URL even tho I set the album to be public

end

def authorize_image
unless current_user.present? && (current_user.site_admin? || logged_in_as_admin_of?(album.circle) || album.shared?)
Copy link
Member

Choose a reason for hiding this comment

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

I think this line would be complete with only checking for album.shared?

Copy link
Member

Choose a reason for hiding this comment

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

Otherwise anytime someone/some website would like to reach the image (for example, our blog), there should be a user logged in

Copy link
Member Author

Choose a reason for hiding this comment

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

oh true, i forgot to check for public albums

Copy link
Member Author

Choose a reason for hiding this comment

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

Will correct it tomorrow

@bbucsy bbucsy requested a review from triszt4n July 11, 2022 17:45
@bbucsy bbucsy merged commit eb99182 into master Jul 11, 2022
@bbucsy bbucsy deleted the feat/short-urls branch July 11, 2022 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace monkeypatch code for private albums Make shorter url
3 participants