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 #38

Merged
merged 10 commits into from
Oct 2, 2024
Merged

GraphQL Migration #38

merged 10 commits into from
Oct 2, 2024

Conversation

stigi
Copy link
Contributor

@stigi stigi commented Oct 1, 2024

Description

Resolves #37

This PR replaces the /graphql endpoint by using /notifications instead.

This PR Removes the Graphql Datasource and replaces it with a standard Get datasource. In order to support pagination and filtering I had to extend the http client to accept query items (via URLQueryItem).

Adjusting the unit tests was quite bothersome, as a lot of them relied on using mocks of graphql data (hello edges and nodes) which had to be recreated. Also the primitives of pagination changed quite significantly.

Breaking changes

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

The API for filtering by topic and category were changed. They previously took arrays of arguments, now they only take singular arguments, as that's what's supported by the /notifications endpoint.

Internal discussion

The API for requesting notifications from the store object changed from calling a function to accessing a property (which is much nicer): store.notifications() to store.notifications().

Drive By fixes:

  1. Updated Example/Example/UIKit/MagicBellStoreViewController.swift to prevent a race condition between the initial refresh and the scroll view triggering a fetch of the next notification page. This caused the first page of notifications to be loaded twice (even before the current PRs changes).

  2. Also updated the Cotnributing section in the Readme to use carthage bootstrap instead of carthage update which would not just resolve the dependencies, but also update them.

Test Plan

Unit tests are run on CI. Additional manual testing steps below.

Setup

  • Clone repo
  • Install dependencies via carthage (make sure to have version 0.40.0 if you're using Xcode 16): carthage bootstrap --use-xcframeworks --no-use-binaries

Running Unit Tests

  • Run tests via swift test
  • Run tests by opening MagicBell.xcodeproj and running tests by selecting MagicBellTests and pressing ⌘+u

Running Example App

  • Install cocoapods in Example: pod install
  • Open Example.xcworkspace (generated by cocoapods) and run the app
  • Manually test: Pagination and Predicates (via the Edit button on the top right) work

@stigi stigi force-pushed the ullrich/37-graphql-migration branch from 3b2bae1 to 9b899bb Compare October 1, 2024 14:37
@stigi stigi force-pushed the ullrich/37-graphql-migration branch from 1dd12fe to c04f317 Compare October 2, 2024 11:05
@stigi stigi force-pushed the ullrich/37-graphql-migration branch from c04f317 to 6dbdc8d Compare October 2, 2024 11:14
@stigi stigi requested a review from smeijer October 2, 2024 11:31
@stigi stigi marked this pull request as ready for review October 2, 2024 11:33
README.md Outdated Show resolved Hide resolved
Co-authored-by: Stephan Meijer <stephan.meijer@gmail.com>
@stigi stigi merged commit db9bfdb into main Oct 2, 2024
1 check passed
@stigi stigi deleted the ullrich/37-graphql-migration branch October 2, 2024 14:01
@stigi stigi mentioned this pull request Oct 2, 2024
3 tasks
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