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.

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

feat: Refactor usage of XHR for Config API #898

Merged

Conversation

alexs-mparticle
Copy link
Collaborator

@alexs-mparticle alexs-mparticle commented Jul 30, 2024

Instructions

  1. PR target branch should be against development
  2. PR title name should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-title-check.yml
  3. PR branch prefix should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-branch-check-name.yml

Summary

  • Replaces XHR Uploader with AsyncUploader, which allows config api requests to support both XHR and Fetch.

Testing Plan

  • Was this tested locally? If not, explain why.
  • Using both self-hosted and CDN sample apps, attempt to load the Web SDK and verify expected behavior.

Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)

@alexs-mparticle alexs-mparticle force-pushed the feat/SQDSDKS-6566-refactor-xhr-config-api branch from 87cc92a to aaf62fe Compare August 20, 2024 18:00
@alexs-mparticle alexs-mparticle marked this pull request as ready for review August 20, 2024 18:46
Co-authored-by: Robert Ing <rmi22186@gmail.com>
Copy link
Member

@rmi22186 rmi22186 left a comment

Choose a reason for hiding this comment

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

very minor nits

src/configAPIClient.ts Outdated Show resolved Hide resolved
src/configAPIClient.ts Show resolved Hide resolved
Comment on lines +171 to +174
mpInstance?.Logger?.verbose(
'Issue with receiving configuration from server, received HTTP Code of ' +
response.statusText
);
Copy link
Member

Choose a reason for hiding this comment

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

i don't think this is ever reached? I think this can be removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If the response.status is anything other than 200, this will be reached.

src/uploaders.ts Outdated Show resolved Hide resolved
alexs-mparticle and others added 2 commits August 21, 2024 09:52
Co-authored-by: Robert Ing <rmi22186@gmail.com>
Copy link

sonarcloud bot commented Aug 21, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
10.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@alexs-mparticle alexs-mparticle merged commit 0b6fae6 into development Aug 26, 2024
24 of 28 checks passed
github-actions bot pushed a commit that referenced this pull request Aug 26, 2024
# [2.28.0](v2.27.0...v2.28.0) (2024-08-26)

### Features

* Refactor usage of XHR for Config API ([#898](#898)) ([0b6fae6](0b6fae6))
@mparticle-automation
Copy link
Collaborator

🎉 This PR is included in version 2.28.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants