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

Expirarion Update #149

Open
OrH-Simplee opened this issue Jan 6, 2022 · 3 comments
Open

Expirarion Update #149

OrH-Simplee opened this issue Jan 6, 2022 · 3 comments

Comments

@OrH-Simplee
Copy link

when generating a shortened URL for the same link, and sending a new expiration date, it doesn't update the expiration.
example:
generate a link today, with 6.month.from_now expiration date with :
shortened_url = Shortener::ShortenedUrl.generate(url, expires_at: 6.month.from_now)
then in 3 months, generate the same link, with the exact same code:
shortened_url = Shortener::ShortenedUrl.generate(url, expires_at: 6.month.from_now)

expectation - expiration would be updated.

important to note - I don't know that the link already exists, but I do want to get the same shortened link. is it on purpose? any way around it?

@fschwahn
Copy link
Collaborator

Confusingly, this even returns an already expired ShortenedUrl in case one exists. Not sure the default behavior can change, but it possibly could be an option passed to generate to optionally update the expires_at-column.

@jpmcgrath
Copy link
Owner

I believe the history here is the generate method was introduced in the original release. If someone asks to generate a shortened url for an existing address, the logic was to return the original record. We provided the fresh option for when you wanted to create a new ShortenedUrl instead of retrieving an existing record. See:

https://github.com/jpmcgrath/shortener#label-Fresh+Links

Then the expires_at option was introduced without taking into account the existing behaviour that returns previously created ShortenedUrls.

Right now you can, with one call:

  1. get the existing record if it exists, without updating, and create a new one if it does not exist
  2. force the generation of a new record with the new expiry date

But you cannot get link if it exists, and update date the expiry if it exists, all in a single call.

If someone provides a new expires_at for an URL that already has a ShortenedUrl, does it mean they want to get the existing record, and update it? Or does it mean they want a completely new ShortenedUrl? Or does it mean they want the existing record, and to keep it as is? I can people expecting any of these as a the "correct and intuitive result". Probably the "least inituitive" is supplying a new expiry, and the resulting expiry is sometimes ignored, but it still makes some sense. Perhaps you don't want to destroy that info?

A workaround here is to do a second step to update the expiry date of your shortened url. ActiveRecord should prevent DB activity when it is not required (e.g. if you provide the same expiry date as was set on a newly generated record).

As for a "fix", I am reluctant to change the existing behaviour now, as people may have come to rely on the current behaviour as a feature. @fschwahn suggestion of a new option could work, or perhaps a configuration option.

@OrH-Simplee
Copy link
Author

@jpmcgrath what about adding a new feature flag on the generate function, that will be false by default, and it will refresh existing exires_at if exists

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

No branches or pull requests

3 participants