Skip to content

refactor: Update api endpoints #51

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.

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

Merged
merged 4 commits into from
Sep 4, 2024
Merged

refactor: Update api endpoints #51

merged 4 commits into from
Sep 4, 2024

Conversation

tjtanjin
Copy link
Member

@tjtanjin tjtanjin commented Aug 31, 2024

Description

The API response has been standardized on the backend, frontend should update to match it.

Closes #46

What change does this PR introduce?

Please select the relevant option(s).

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • CI/CD (updates related to the CI/CD process)
  • Documentation update (changes to docs/code comments)
  • Chore (miscellaneous tasks that do not fall into the above options)

What is the proposed approach?

Update handling of API responses based on standardized format.

Checklist:

  • The commit message follows our adopted guidelines
  • Testing has been done for the change(s) added (for bug fixes/features)
  • Relevant comments/docs have been added/updated (for bug fixes/features)

@tjtanjin tjtanjin requested a review from NeonNature September 4, 2024 17:14
} else {
apiThemes = Placeholders.themes;
}

const themes = await fetchThemesFromGitHub(apiThemes);
setThemes(themes);
if (apiThemes) {
Copy link
Contributor

@NeonNature NeonNature Sep 4, 2024

Choose a reason for hiding this comment

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

I might be missing something here but wouldn't apiThemes always be present/not null because it's either the API response or the placeholders?

could also initialize with let apiThemes = Placeholder.themes instead of null if that's the case too. or is there a case the response could be null?

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right that the response can actually be null - if success is false then the data field will be absent 😝

Copy link
Contributor

Choose a reason for hiding this comment

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

gotcha. Looks good to me, if so.

@tjtanjin tjtanjin merged commit 5cabfac into main Sep 4, 2024
1 check passed
@tjtanjin tjtanjin deleted the update-api-endpoints branch June 1, 2025 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task] Update frontend API calls to integrate with standardized schema response from backend
2 participants