Skip to content

Prevent race condition between prevenTimeout and show modal#9092

Merged
ahukkanen merged 1 commit intodecidim:developfrom
mainio:fix/timeout_race_condition
Apr 7, 2022
Merged

Prevent race condition between prevenTimeout and show modal#9092
ahukkanen merged 1 commit intodecidim:developfrom
mainio:fix/timeout_race_condition

Conversation

@lahdeero
Copy link
Copy Markdown
Contributor

🎩 What? Why?

After #9070 there is race condition between preventing timeout and showing timeout modal. So if user has meeting going on in tab A and homepage open in tab B, tab B could still show logout modal even if tab A is preventing timeout.

📌 Related Issues

#9070
#9091

Testing

  1. (OPTIONAL) Change expire_session_after default time (in minutes) from development_app/config/secrets.yml
  2. Create online meeting with embedded url
  3. Go to that meetings page
  4. Open another tab and go to some other url (e.g. /admin)
  5. There was 50% change that /admin shows timeout modal.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

♥️ Thank you!

@lahdeero
Copy link
Copy Markdown
Contributor Author

Can bet tested @ https://eero.populihub.app/processes/curriculum-ambiguous/f/12/meetings/73 there is 3min timeout delay.

@andreslucena
Copy link
Copy Markdown
Member

Tried it locally and I see the behavior described:

Logged in as admin, with a live ongoing meeting and another tab opened with /admin, I see the timeout modal in this tab after a couple of minutes (I have DECIDIM_EXPIRE_SESSION_AFTER=2).

@andreslucena andreslucena added module: core type: fix PRs that implement a fix for a bug labels Apr 4, 2022
@ahukkanen ahukkanen merged commit 7d71042 into decidim:develop Apr 7, 2022
@ahukkanen ahukkanen deleted the fix/timeout_race_condition branch April 7, 2022 07:46
@ahukkanen
Copy link
Copy Markdown
Contributor

Can you backport to 0.26 @lahdeero 🙏

@lahdeero
Copy link
Copy Markdown
Contributor Author

lahdeero commented Apr 7, 2022

@ahukkanen 0.26 already has this, I found this bug when testing previous backport.

entantoencuanto added a commit that referenced this pull request Apr 7, 2022
* develop:
  Compile SCSS through sass-embedded (#9081)
  Prevent race condition between prevenTimeout and show modal (#9092)
  Bump elections dependencies to 0.23.0 (#9140)
  Reduce d3 bundle size (#9034)
  Improve wording when casting your vote (#9098)
  Do not send upcoming meeting notification for hidden or withdrawn meetings (#9134)
  Fix processes count in processes group title cell (#9087)
  Fix attachments when called from Cells (#9136)
  Clarify message to user when checking census (#9112)
@alecslupu alecslupu added this to the 0.27.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: core type: fix PRs that implement a fix for a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants