Skip to content

Refactoring to avoid race condition and resolve CodeQL returnless function alert 56#6543

Merged
drakenguyen4000 merged 2 commits intohackforla:gh-pagesfrom
ajb176:fix-returnless-5473
Mar 31, 2024
Merged

Refactoring to avoid race condition and resolve CodeQL returnless function alert 56#6543
drakenguyen4000 merged 2 commits intohackforla:gh-pagesfrom
ajb176:fix-returnless-5473

Conversation

@ajb176
Copy link
Member

@ajb176 ajb176 commented Mar 28, 2024

Fixes #5473

What changes did you make?

  • Replaced a returnless function with an anonymous function to avoid CodeQL alert 56
  • Added an if conditional to set up an event listener only if the document is still in the loading state
  • Added an else clause to run InsertEventSchedule if the document is not in the loading state

Why did you make the changes (we will use this info to test)?

  • Because the original function passed to the addEventListener contained parameters, the function was called immediately and the event listener expected its parameter to be that function's return value. Using an anonymous function (without parameters) to then call the InsertEventSchedule ensured the event listener only fired the function once the necessary event occured.
  • The previous code was only functional because the event listener was set up incorrectly, otherwise the event listener would have missed the DOMContentLoaded event and never run its callback function. This made it necessary to re-format the script to load the event schedule without the race condition of the DOM content.
  • If the document readyState is still loading, the event listener can be added to wait for the DOMContentLoaded event before populating the event schedule. If the readyState is past the loading state ('interactive'), the DOM content has already loaded and the event schedule can be populated.

Video of Proposed Changes to The Website

Visuals before changes are applied
beforechangescodeql.mov
Visuals after changes are applied
codeqlshortafterchange.mov

@ajb176 ajb176 requested review from W1LD-R and aqandrew March 28, 2024 20:04
@github-actions
Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b ajb176-fix-returnless-5473 gh-pages
git pull https://github.com/ajb176/website.git fix-returnless-5473

Note that CONTRIBUTING.md cannot previewed locally; rather it should be previewed at this URL:

https://github.com/ajb176/website/blob/fix-returnless-5473/CONTRIBUTING.md  

Copy link
Member

@W1LD-R W1LD-R left a comment

Choose a reason for hiding this comment

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

Hi @ajb176 — nicely done!

  • I like your solution, it's concise and it works as expected when I view the site on your branch
  • Your branch is correct
  • You linked the issue correctly in your PR description
  • You clearly described the changes, stated the reason for making them, and provided before/after gifs
  • Everything looks good across mobile and desktop on Chrome and Firefox

Approved! 👍

aqandrew added a commit to aqandrew/website that referenced this pull request Mar 28, 2024
Copy link
Member

@aqandrew aqandrew 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 exactly as expected, and your explanation is very sound! Great job.

Also, thank you for sharing your approach for the same problem in #6536.

drakenguyen4000 pushed a commit that referenced this pull request Mar 31, 2024
…ng meetings (#6536)

* refactor: use anonymous function as event listener callback for listing meetings

* fix: prevent race condition between DOMContentLoaded and getEventData

based on #6543
@drakenguyen4000 drakenguyen4000 merged commit b6da63a into hackforla:gh-pages Mar 31, 2024
freaky4wrld pushed a commit to freaky4wrld/website that referenced this pull request Apr 9, 2024
…ng meetings (hackforla#6536)

* refactor: use anonymous function as event listener callback for listing meetings

* fix: prevent race condition between DOMContentLoaded and getEventData

based on hackforla#6543
freaky4wrld pushed a commit to freaky4wrld/website that referenced this pull request Apr 9, 2024
…ction alert 56 (hackforla#6543)

* Possible fix for returnless function

* Fix for returnless function
@ajb176 ajb176 deleted the fix-returnless-5473 branch May 29, 2024 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complexity: Medium Feature: Code Alerts role: back end/devOps Tasks for back-end developers role: front end Tasks for front end developers size: 1pt Can be done in 4-6 hours

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Resolve CodeQL query alert 56

4 participants