Skip to content

Conversation

NGPixel
Copy link
Member

@NGPixel NGPixel commented Apr 3, 2025

No description provided.

@NGPixel NGPixel requested review from rjsparks and Copilot April 3, 2025 14:52
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request fixes the fetch call for agenda data by removing the unnecessary credentials option, streamlining the API request.

  • Removed "credentials: 'omit'" from the fetch call.
  • Simplified the request to better align with the API's expected behavior.

Copy link
Member

@jennifer-richards jennifer-richards left a comment

Choose a reason for hiding this comment

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

Wonder if it should be 'same-origin' to reduce risk of CSRF issues if the code gets reused?

https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch#including_credentials

edit: never mind, I see that's the default if I actually read my own reference 😬

@rjsparks rjsparks marked this pull request as draft April 15, 2025 15:15
@rjsparks
Copy link
Member

From discussion - instead of this PR we probably want to either change the code to make production and staging behave differently, or change the CF rules around the api endpoint at staging instead.

If we want to do that in this code, please update this PR, otherwise close it.

@rjsparks rjsparks marked this pull request as ready for review April 15, 2025 15:44
@rjsparks
Copy link
Member

Subsequent discussion convinced me I was incorrect above.

@rjsparks rjsparks merged commit 86988eb into main Apr 15, 2025
8 checks passed
@NGPixel NGPixel deleted the fix-agenda-fetch-creds branch April 15, 2025 15:48
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants