Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Added missing deleted? method to cookie jar. #1040

Merged
merged 1 commit into from
Mar 13, 2020
Merged

Conversation

jwoertink
Copy link
Member

Purpose

Fixes #963

Description

In the guides we make mention of a cookies.deleted?() method that never actually existed. I'm not sure where it came from (probably me), but this adds that method in.

Deleting cookies is weird because to delete a cookie, you set the value to an empty string, and the expire time to something in the past. It's actually up to the individual browsers to remove that cookie. This deleted? method will return true if the cookie is both expired? && value.blank? otherwise it's just false.

Side note: in the guides, we also make mention of session.deleted?(:name). I decided to not implement this because if a session key is deleted, then it's just nil. I'm ok with adding it for consistency in the API, but initially I didn't think it's necessary.

Checklist

  • - An issue already exists detailing the issue/or feature request that this PR fixes
  • - All specs are formatted with crystal tool format spec src
  • - Inline documentation has been added and/or updated
  • - Lucky builds on docker with ./script/setup
  • - All builds and specs pass on docker with ./script/test

Copy link
Member

@paulcsmith paulcsmith left a comment

Choose a reason for hiding this comment

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

Looks good! And I agree on the session.deleted?. Since it is stored as a JSON object it is basically the same as checking if it is nil so we can remove that from the guides

@jwoertink jwoertink merged commit b19f018 into master Mar 13, 2020
@jwoertink jwoertink deleted the features/963 branch March 13, 2020 14:50
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing cookie and session methods from the docs
2 participants