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

GraphQL Migration #25

Merged
merged 10 commits into from
Oct 9, 2024
Merged

GraphQL Migration #25

merged 10 commits into from
Oct 9, 2024

Conversation

stigi
Copy link
Member

@stigi stigi commented Oct 8, 2024

Change description

Resolves #19

Note

This PR depends on other PRs and needs to be rebased on main before merge:

This PR updates the SDK and removes all traces of the /graphql endpoint. Notifications are now fetched via the /notifications endpoint. It's in line with magicbell/magicbell-swift#38 in the Swift SDK.

Breaking changes

⚠️ This PR contains breaking changes and will be released with a major version bump:

The breaking changes are mostly around deprecation of topics and categories in favor of their singulars topic and category.

Also the switch away from graphql is major enough that it might justify a major version change. Bumping a major version will also keep the SDKs version in sync with the Swift SDKs version.

Drive By fixes

  • I noticed that the existing code had a bug that would prevent the category filter from working. This bug is now fixed.
  • Updated some copy pasta in the readme: replaced nil with null (because we're in Kotlin land, not Swift)

Test Plan

Unit tests are run on CI. New tests to verify behaviour of query parameters have been added

Additional manual testing by pointing the example app to a user with 150 notifications. Verified that loading, paging and predicates work.

Type of Change

  • Bug fix
  • Feature
  • Enhancement

Guidelines

  • A changeset is included, or the change is not noteworthy enough to warrant one

@stigi stigi changed the base branch from main to ullrich/ci-lint October 8, 2024 20:17
@stigi stigi force-pushed the ullrich/remove-graphql branch 6 times, most recently from fe04702 to c6abb69 Compare October 9, 2024 12:53
@stigi stigi changed the title Remove GraphQL GraphQL Migration Oct 9, 2024
@stigi stigi requested a review from smeijer October 9, 2024 13:08
@stigi stigi marked this pull request as ready for review October 9, 2024 13:09
Base automatically changed from ullrich/ci-lint to main October 9, 2024 14:13
@stigi stigi force-pushed the ullrich/remove-graphql branch from 6329508 to 15801ed Compare October 9, 2024 14:14
@stigi
Copy link
Member Author

stigi commented Oct 9, 2024

^^ rebased on main after merging #23. No code changes

@stigi stigi merged commit f3915c7 into main Oct 9, 2024
2 checks passed
@stigi stigi deleted the ullrich/remove-graphql branch October 9, 2024 14:19
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.

Migrate off of graphql endpoint
2 participants