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.

Sign up for GitHub

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

Jump to bottom

[34] Timeout Warning #115

Merged
merged 17 commits into from
Aug 27, 2024
Merged

[34] Timeout Warning #115

merged 17 commits into from
Aug 27, 2024

Conversation

cpreisinger
Copy link
Contributor

#34

@cpreisinger cpreisinger requested a review from stepchud August 10, 2024 17:52
@cpreisinger cpreisinger self-assigned this Aug 10, 2024
stepchud
stepchud reviewed Aug 14, 2024
View reviewed changes
Copy link
Contributor

@stepchud stepchud left a comment

Choose a reason for hiding this comment

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

forgot to submit this comment earlier

app/views/layouts/application.html.erb Outdated Show resolved Hide resolved
stepchud
stepchud reviewed Aug 15, 2024
View reviewed changes
Copy link
Contributor

@stepchud stepchud left a comment

Choose a reason for hiding this comment

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

nothing major just some small comments or suggestions

app/controllers/application_controller.rb Outdated Show resolved Hide resolved
app/javascript/application.js Outdated Show resolved Hide resolved
app/javascript/application.js Outdated Show resolved Hide resolved
app/controllers/application_controller.rb Outdated Show resolved Hide resolved
@stepchud
Copy link
Contributor

I'm seeing the timeout warning even when I'm logged out already. It says your session will expire in without details.

Screenshot 2024-08-14 at 11 00 26 PM

cpreisinger and others added 4 commits August 16, 2024 14:31
@cpreisinger
34 Timeout redirect, user activity, and countdown
f806d14
@cpreisinger
Merge branch 'dev' of github.com:GSA/Challenge_platform into 34/timeo…
cdff3a0 
…ut-warning

* 'dev' of github.com:GSA/Challenge_platform:
  add two more utility classes since uswds is missing these
  Updated links in footer
  Update app/views/layouts/_footer.html.erb
  Update app/views/layouts/_footer.html.erb
  added dashboard spec
  Added shared_examples for footer; testing on / route
  tablet logo order, margin-y
  last layout tweaks
  updated comment
  update the social-icons svg path and tweak the layout
  Updated icon colors and footer responsiveness
  changed icon color to primary darker
  wip challenge  footer
@cpreisinger
34 Revert tool-versions file
c198e1e
@stepchud
Merge branch 'dev' into 34/timeout-warning
cebbd9e
stepchud
stepchud reviewed Aug 19, 2024
View reviewed changes
Copy link
Contributor

@stepchud stepchud left a comment •
edited
Loading

Choose a reason for hiding this comment

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

seems like it is working locally except for the countdown rounding error. it needs some additional configuration for deployment.

.tool-versions
@@ -1,3 +1,3 @@
ruby 3.2.4
ruby 3.2.4
Copy link
Contributor

Choose a reason for hiding this comment

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

whitespace

Suggested change
ruby 3.2.4
ruby 3.2.4

.envrc Outdated Show resolved Hide resolved
app/javascript/time_helpers.js Outdated Show resolved Hide resolved
@cpreisinger
34 Fix session expiration condition and countdown
30c8b57
@cpreisinger
Merge branch '34/timeout-warning' of github.com:GSA/Challenge_platfor…
16be079 
…m into 34/timeout-warning

* '34/timeout-warning' of github.com:GSA/Challenge_platform:
  Bump rails from 7.1.3.4 to 7.2.0
  Bump ruby-lsp from 0.17.11 to 0.17.13
  change dependabot to monitor dev branch
@cpreisinger
34 Fix failing session timeout test
f7ba233
stepchud
stepchud reviewed Aug 23, 2024
View reviewed changes
.envrc Outdated Show resolved Hide resolved
@cpreisinger @stepchud
34 Remove session timeout from .envrc
d50cefc 
Co-authored-by: Stephen Chudleigh <stepchud@users.noreply.github.com>
stepchud
stepchud requested changes Aug 23, 2024
View reviewed changes
Copy link
Contributor

@stepchud stepchud left a comment

Choose a reason for hiding this comment

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

Seeing some buggy behavior when I leave the browser open and let it timeout on its own. The countdown keeps going after 0 and never redirects to the logged out page until I refresh the browser manually.

Screen.Recording.2024-08-23.at.3.24.18.PM.mov

@stepchud
Copy link
Contributor

Seeing some buggy behavior when I leave the browser open and let it timeout on its own. The countdown keeps going after 0 and never redirects to the logged out page until I refresh the browser manually.

I logged some console output and it looks like the request to DELETE /sessions/timeout happens, which results in a redirect, immediately followed by a DELETE /dashboard which is the 404 and for some reason what's logged to the console.
Screenshot 2024-08-23 at 3 44 15 PM

Screenshot 2024-08-23 at 4 22 09 PM Screenshot 2024-08-23 at 4 22 13 PM

@cpreisinger
34 Fix for timeout not redirecting properly
946afaa
@cpreisinger
Merge branch '34/timeout-warning' of github.com:GSA/Challenge_platfor…
d6ff17e 
…m into 34/timeout-warning

* '34/timeout-warning' of github.com:GSA/Challenge_platform:
  34 Remove session timeout from .envrc
@cpreisinger cpreisinger requested a review from stepchud August 27, 2024 14:44
stepchud
stepchud reviewed Aug 27, 2024
View reviewed changes
Copy link
Contributor

@stepchud stepchud left a comment

Choose a reason for hiding this comment

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

this works for me, just a small change on the before_action

app/controllers/sessions_controller.rb Outdated Show resolved Hide resolved
cpreisinger and others added 2 commits August 27, 2024 14:10
@cpreisinger @stepchud
34 Use skip before action instead of except for timeout
96e444a 
Co-authored-by: Stephen Chudleigh <stepchud@users.noreply.github.com>
@cpreisinger
Merge branch 'dev' into 34/timeout-warning
b7f1c15
@stepchud stepchud linked an issue Aug 27, 2024 that may be closed by this pull request
Timeout Warning #34
Closed
41 tasks
@stepchud stepchud added this to the Sprint 08/27/24 milestone Aug 27, 2024
stepchud
stepchud approved these changes Aug 27, 2024
View reviewed changes
Copy link
Contributor

@stepchud stepchud 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 to me

@cpreisinger cpreisinger merged commit f904630 into dev Aug 27, 2024
11 checks passed
@cpreisinger cpreisinger deleted the 34/timeout-warning branch August 27, 2024 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stepchud stepchud stepchud approved these changes

Assignees

@cpreisinger cpreisinger

Labels
None yet
Projects
None yet
Milestone
Sprint 08/27/24
Development

Successfully merging this pull request may close these issues.

Timeout Warning
2 participants
@cpreisinger @stepchud