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

Store browser sessions in database #2482

Merged
merged 4 commits into from
Nov 18, 2024
Merged

Store browser sessions in database #2482

merged 4 commits into from
Nov 18, 2024

Conversation

thomasleese
Copy link
Contributor

This changes the session store to use an Active Record one, allowing us to store more data in the session than the cookie store allows us to.

Although we're not seeing problems at the moment, I think it's worth doing now as we're starting to create draft vaccination records and consents in the session, there's a risk the cookie will get too big.

We can use this Gem to store sessions in the database allowing for more
than 4 KB of data, which is useful now that we're starting to create
consents and vaccination records in the session.
This adds a new table which we'll use to store sessions backed by
ActiveRecord rather than cookies, allowing us to hold data larger than 4
KB.
This allows us to store more data in the session than the 4 KB limit of
the cookie store, which might be useful particularly as we're starting
to create draft consents and vaccination records in the session.
This mimicks the behaviour of a Rake task which automatically removes
old sessions from our database. We want to run this on a schedule and
using a job seemed like the easiest way to do it.
Copy link

sonarcloud bot commented Nov 18, 2024

@misaka
Copy link
Contributor

misaka commented Nov 18, 2024

Somewhere, maybe the SLSP, we documented that we store data on the client browser in the cookie, and had to explain that it's encrypted, etc. Gotta love over-documentation.

@tvararu tvararu temporarily deployed to mavis-pr-2482 November 18, 2024 09:55 Inactive
Copy link
Contributor

@benilovj benilovj left a comment

Choose a reason for hiding this comment

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

Somewhere, maybe the SLSP, we documented that we store data on the client browser in the cookie, and had to explain that it's encrypted, etc. Gotta love over-documentation.

Can we hold off merging for now for this reason?

@thomasleese thomasleese merged commit b386dd9 into main Nov 18, 2024
12 checks passed
@thomasleese thomasleese deleted the database-backed-store branch November 18, 2024 10:21
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.

4 participants