Session timeout javascript #295

jinnashravan · 2025-01-28T20:47:37Z

This is Javascript version, where we show a session timeout warning banner.
After 15 minutes, we show a warning. If the user clicks continue we extend the session for another 20 minutes.
If no activity happens in 20 minutes, we show a sesssion ended screen.

1. After 20 minutes cookie session timesout and shows a session-ended page.

Session Timeout Javascript verion In settings session cookie age is set to 20 minutes. After 15 minutes, show a warning dialog about session expiry After 20 minutes, show the session ended page.

Using the clock and tick API's of cypress to override the time function. Warning appears after 15 minutes and session ends after 20 minutes if no action is taken.

dhvander · 2025-01-29T10:50:28Z

request_a_govuk_domain/request/templates/registrar_details.html

+                <div class="govuk-notification-banner__content">
+                  <h2 class="govuk-heading-s">Your session can time out</h2>
+                <div class="govuk-body">
+                  <p>We will delete your answers if you do not do anything for 20 minutes and you will have to start again.</p>


Should the 20 be read from the settings value?

dhvander · 2025-01-29T10:50:53Z

request_a_govuk_domain/request/templates/session_ended.html

+            </h1>
+
+            <p class="govuk-body">
+                Your session has ended because you haven't done anything for over 20 minutes.


Same as above

dhvander · 2025-01-29T10:51:33Z

request_a_govuk_domain/request/templates/session_ended.html

+{% load csp %}
+
+{% block title %}Session Ended{% endblock %}
+{% block session_ended %}{% endblock %}


Maybe add a comment here to say why you have overidden the block

dhvander · 2025-01-29T10:52:21Z

request_a_govuk_domain/request/views.py

+
+
+def reset_timer(request):
+    logger.info("Resetting the session timer")


Remove the log statements?

dhvander · 2025-01-29T10:54:10Z

request_a_govuk_domain/request/templates/base.html


 </body>
+{% block session_ended %}
+<script type="text/javascript" nonce="{{ request.csp_nonce }}">


Add comments and remove log statements that are not needed

cypress/e2e/session_timeout.cy.js

maxf · 2025-01-29T15:13:17Z

request_a_govuk_domain/request/static/request.css

-  background: rgb(243, 242, 241);
-  color: rgb(209, 49, 24);
-  font-size: 16px;
+    background: rgb(243, 242, 241);


Please fix all your indentation. It should be 2 white space chars everywhere for CSS, HTML and JS

request_a_govuk_domain/request/forms.py

maxf · 2025-01-29T15:15:44Z

request_a_govuk_domain/request/templates/base.html

    {% endif %}
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1, viewport-fit=cover">
    <meta name="theme-color" content="#0b0c0c">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
+    <!-- Sets the auto referesh to match with session cookie age which is 20 minutes -->
+    <!-- <meta http-equiv="refresh" content="1200; url=/session-ended/"> -->


Why is this removed? How does it work when JS is disabled?

Removed log messages and using SESSION_TIMEOUT param from setting in HTML. Updated Cypress tests to check for model_dialog not be visible at start.

maxf · 2025-01-30T08:58:31Z

request_a_govuk_domain/request/templates/base.html

@@ -1,19 +1,22 @@
 {% load static i18n govuk_frontend_django csp %}<!DOCTYPE html>
 <html lang="en" class="govuk-template">
-  <head>
+<head>


Please don't break the indentation unless there's a good reason. Otherwise it's very difficult to check what's actually changed

maxf · 2025-01-30T08:59:44Z

request_a_govuk_domain/request/templates/base.html


 </body>
+{% block session_ended %} <!-- session-ended block to override in child template -->
+<script type="text/javascript" nonce="{{ request.csp_nonce }}">
+  var warningTimerId = 0;


Please use const or let. var should be avoided these days

maxf · 2025-01-30T09:00:12Z

request_a_govuk_domain/request/templates/base.html

+
+  // Show warning dialog 15 minutes before session expiry
+  function showWarning() {
+      var warningDiv = document.getElementById('modal_dialog');


Please use 2 whitespace chars for indentation, as elsewhere

maxf · 2025-01-30T09:00:45Z

request_a_govuk_domain/request/templates/session_ended.html

+
+{% block content %}
+{% load custom_filters %}
+    <div class="govuk-grid-row">


maxf · 2025-01-30T09:02:15Z

request_a_govuk_domain/request/templatetags/custom_filters.py

+
+
+@register.filter
+def divide_by_60(value):


type hints would be useful here and wherever you can add them in your new code

maxf · 2025-01-31T09:47:39Z

request_a_govuk_domain/request/templates/base.html

+  function resetTimers() {
+    clearTimeout(warningTimerId);
+    clearTimeout(expiryTimerId);
+    let sessionCookieAge = "{{ SESSION_TIMEOUT|safe }}";


it should be a const and you should remove the quote marks as it's supposed to be a number, not a string.

maxf · 2025-01-31T09:52:36Z

request_a_govuk_domain/request/templates/base.html

+    clearTimeout(warningTimerId);
+    clearTimeout(expiryTimerId);
+    let sessionCookieAge = "{{ SESSION_TIMEOUT|safe }}";
+    warningTimerId = setTimeout(showWarning, (sessionCookieAge - 300) * 1000); // 15 minutes


The warning timeout duration should also be a setting like SESSION_TIMEOUT

maxf · 2025-01-31T10:14:38Z

request_a_govuk_domain/request/templates/session_ended.html

+{% load custom_filters %}
+  <div class="govuk-grid-row">
+    <div class="govuk-grid-column-two-thirds">
+        {% error_summary form %}


Why is this a form. Can the button just link to the start page (if that resets the session)?

maxf · 2025-01-31T10:36:30Z

cypress/e2e/session_timeout.cy.js

+    cy.get('.govuk-grid-column-two-thirds > .govuk-button').click();
+    cy.get('#id_registrar_organisation').clear().type('WeRegister')
+    cy.get('#id_registrar_name').clear('Test');
+    cy.get('#id_registrar_phone').clear('01225672345');
+    cy.get('#id_registrar_email').clear('[email protected]');
+    cy.get('#id_submit').click();


I think you could reduce this to 1 line with goToRegistrantType

maxf · 2025-01-31T10:38:12Z

cypress/e2e/session_timeout.cy.js

+    cy.start()
+  });
+
+  it('session_timeout_dialog_success', () => {


This string is supposed to be human-readable when looking at test results to understand what the test does. Here you could use it('Displays the timeout warning after 15 minutes', for instance

maxf · 2025-01-31T10:39:11Z

cypress/e2e/session_timeout.cy.js

+    cy.get('.govuk-grid-column-two-thirds > .govuk-button').click();
+    cy.get('#id_registrar_organisation').clear().type('WeRegister')
+    cy.get('#id_registrar_name').clear('Test');
+    cy.get('#id_registrar_phone').clear('01225672345');
+    cy.get('#id_registrar_email').clear('[email protected]');
+    cy.get('#id_submit').click();


Same as above, use the utility function.

maxf · 2025-01-31T10:40:23Z

cypress/e2e/session_timeout.cy.js

+    cy.get('#modal_dialog').should('not.be.visible');
+
+    // Check if the user is not redirected to the session-ended page
+    cy.url().should('include', '/session-ended/');


Add an extra step to check if when the Continue button is clicked you go back to the start page.

maxf · 2025-01-31T10:41:15Z

cypress/e2e/session_timeout.cy.js

+
+  });
+
+  it('session_timeout_session_ended', () => {


Did you try a test with multiple tabs? Since this is what justified the server-side timeout, we should have an example where the multi-tab handling works.

request_a_govuk_domain/request/forms.py

maxf · 2025-01-31T10:44:32Z

request_a_govuk_domain/request/templates/base.html

+            <p>We will delete your answers if you do not do anything for {{ SESSION_TIMEOUT|divide_by_60 }} minutes and you will have to start
+                again.</p>
+        </div>
+        <button class="govuk-button dialog-button js-dialog-close" data-module="govuk-button" id="session-continue">Continue</button>


is this data-module attribute used?

Improved on cypress tests.

maxf · 2025-02-04T09:30:29Z

cypress/e2e/session-timeout.cy.js

+    cy.clock();
+  });
+
+  it('Displays the timeout warning after 15 minutes', () => {


Now that the timeout duration is a parameter you should change the description to something like
"displays the timeout warning after the preset duration".

maxf · 2025-02-04T09:32:39Z

cypress/e2e/session-timeout.cy.js

+    // check if model_dialog doesn't exist
+    cy.get('#modal_dialog').should('not.be.visible');
+    // clock tick
+    cy.tick(15 * 60 * 1000);


Tricky here as you can't reuse SESSION_COOKIE_AGE. However you could redeclare the same variables at the top of this file, with the same value as in the python code, or different ones, it doesn't matter.

maxf · 2025-02-04T09:33:10Z

cypress/e2e/session-timeout.cy.js

+    cy.goToRegistrantType()
+
+    // clock tick
+    cy.tick(15 * 60 * 1000);


Same. Use a constant. Also a few times in your code below.

maxf · 2025-02-04T09:34:21Z

cypress/e2e/session-timeout.cy.js

+  it('Displays the timeout warning after 15 minutes and continue the session', () => {
+    // go to RegistrantType page
+    cy.goToRegistrantType()
+


Just check that the modal dialog isn't visible at this point. It makes the test easier to understand: 1. check it's not visible, 2. let time pass, 3. check if it's visible

maxf · 2025-02-04T09:36:26Z

request_a_govuk_domain/request/templates/base.html

@@ -148,7 +150,64 @@
    <script nonce="{{request.csp_nonce}}" src='{% static "js.cookie.min.js" %}'></script>
    <script nonce="{{request.csp_nonce}}" src='{% static "cookies.js" %}'></script>
  {% endif %}
-
+  {% load custom_filters %}
+  <div id="modal_dialog" class="modal">


Can you use a more specific id, like "timeout-warning-modal" ? Also as a reminder, the convention for html ids is to use hyphens not underscores if possible

maxf · 2025-02-04T09:53:31Z

request_a_govuk_domain/request/templates/base.html

+
+  // Show warning dialog 15 minutes before session expiry
+  function showWarning() {
+      let warningDiv = document.getElementById('modal_dialog');


2 character indent please

maxf · 2025-02-04T09:54:03Z

request_a_govuk_domain/request/templates/registrar_details.html

+    <div class="govuk-grid-column-two-thirds">
+      {% error_summary form %}
+      <h1 class="govuk-heading-xl">
+          Registrar details


2 character indent please

maxf · 2025-02-04T09:57:36Z

request_a_govuk_domain/request/templates/base.html

@@ -14,6 +14,8 @@
    <meta name="viewport" content="width=device-width, initial-scale=1, viewport-fit=cover">
    <meta name="theme-color" content="#0b0c0c">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
+    <!-- Sets the auto referesh to match with session cookie age which is 20 minutes -->
+    <meta http-equiv="refresh" content="1200; url=/session-ended/">


use the variable

You should put this meta tag in a noscript tag I think. Otherwise, if the user has JS there's a risk of both timeout mechanisms to interfere.

maxf · 2025-02-04T10:03:02Z

request_a_govuk_domain/request/templates/registrar_details.html

+          </h2>
+          </div>
+          <div class="govuk-notification-banner__content">
+              <h2 class="govuk-heading-s">Your session can time out</h2>


Please make sure your indentation is correct everywhere once and for all, so I don't have to remind you multiple times.

maxf · 2025-02-04T10:04:09Z

request_a_govuk_domain/request/views.py

+
+
+def reset_timer(request):
+    return HttpResponse()


use a docstring, otherwise this function might seem confusing.

Session timeout non JS version.

71619a0

1. After 20 minutes cookie session timesout and shows a session-ended page.

jinnashravan requested review from maxf and dhvander January 28, 2025 20:47

Base template with session timeout popup model dialog.

c32dcab

Session Timeout Javascript verion In settings session cookie age is set to 20 minutes. After 15 minutes, show a warning dialog about session expiry After 20 minutes, show the session ended page.

jinnashravan force-pushed the session_timeout_javascript branch from 6cd5a31 to 342f408 Compare January 28, 2025 21:09

Cypress tests for session timeout.

f308e4c

Using the clock and tick API's of cypress to override the time function. Warning appears after 15 minutes and session ends after 20 minutes if no action is taken.

jinnashravan force-pushed the session_timeout_javascript branch from 2570a52 to f308e4c Compare January 28, 2025 21:34

dhvander reviewed Jan 29, 2025

View reviewed changes

maxf requested changes Jan 29, 2025

View reviewed changes

maxf force-pushed the session_timeout_javascript branch from f308e4c to b28cb2b Compare January 29, 2025 17:04

jinnashravan force-pushed the session_timeout_javascript branch from b28cb2b to f308e4c Compare January 29, 2025 20:53

jinnashravan added 2 commits January 29, 2025 20:54

Merge branch 'main' into session_timeout_javascript

45e4fd3

Based on PR comments, updated the indentation in CSS, JS and HTML.

2cb0029

Removed log messages and using SESSION_TIMEOUT param from setting in HTML. Updated Cypress tests to check for model_dialog not be visible at start.

maxf requested changes Jan 30, 2025

View reviewed changes

jinnashravan added 2 commits January 30, 2025 15:02

CSS changes for display the model in the middle of the screen.

51cac03

Fixing Indentation and added type hints for template tags.

225cb3d

maxf requested changes Jan 31, 2025

View reviewed changes

jinnashravan added 2 commits February 3, 2025 05:15

Reverting the indentation to it's original form.

f5a9088

WARNING_TIMEOUT_DURATION as a seeting flag.

46108d9

Improved on cypress tests.

maxf requested changes Feb 4, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Session timeout javascript #295

Session timeout javascript #295

jinnashravan commented Jan 28, 2025

dhvander Jan 29, 2025

dhvander Jan 29, 2025

dhvander Jan 29, 2025

dhvander Jan 29, 2025

dhvander Jan 29, 2025

maxf Jan 29, 2025

maxf Jan 29, 2025

maxf Jan 30, 2025

maxf Jan 30, 2025

maxf Jan 30, 2025

maxf Jan 30, 2025

maxf Jan 30, 2025

maxf Jan 31, 2025

maxf Jan 31, 2025

maxf Jan 31, 2025

maxf Jan 31, 2025

maxf Jan 31, 2025

maxf Jan 31, 2025

maxf Jan 31, 2025

maxf Jan 31, 2025

maxf Jan 31, 2025

maxf Feb 4, 2025

maxf Feb 4, 2025

maxf Feb 4, 2025

maxf Feb 4, 2025

maxf Feb 4, 2025

maxf Feb 4, 2025

maxf Feb 4, 2025

maxf Feb 4, 2025

maxf Feb 4, 2025

maxf Feb 4, 2025



		def reset_timer(request):
		logger.info("Resetting the session timer")

Session timeout javascript #295

Are you sure you want to change the base?

Session timeout javascript #295

Conversation

jinnashravan commented Jan 28, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment