Skip to content

PeriodType/PurchaseOwnershipType/Store: conform to Encodable#1551

Merged
NachoSoto merged 1 commit into
mainfrom
types-encodable
May 10, 2022
Merged

PeriodType/PurchaseOwnershipType/Store: conform to Encodable#1551
NachoSoto merged 1 commit into
mainfrom
types-encodable

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

These will be used in an upcoming PR.

@NachoSoto

Copy link
Copy Markdown
Contributor Author

I should add tests for these.

@joshdholtz joshdholtz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me!

@NachoSoto

Copy link
Copy Markdown
Contributor Author

Adding tests in a future PR because I want to use a helper method added in #1537.

@NachoSoto NachoSoto merged commit 30c1973 into main May 10, 2022
@NachoSoto NachoSoto deleted the types-encodable branch May 10, 2022 15:47
@NachoSoto

Copy link
Copy Markdown
Contributor Author

Added tests: #1558

NachoSoto added a commit that referenced this pull request May 11, 2022
Follow up to #1551. Using `Codable.encodeAndDecode` we ensure that each value can be bidirectionally converted.
These tests exposed that unknown values were being encoded as `null`, but then failed to decode.

I've updated the implementation and added a few more tests to cover this behavior.
NachoSoto added a commit that referenced this pull request May 19, 2022
Follow up to #1551. Using `Codable.encodeAndDecode` we ensure that each value can be bidirectionally converted.
These tests exposed that unknown values were being encoded as `null`, but then failed to decode.

I've updated the implementation and added a few more tests to cover this behavior.
NachoSoto added a commit that referenced this pull request May 24, 2022
### Depends on:
- #1537
- #1540
- #1541
- #1543
- #1546
- #1547
- #1550
- #1551,
- #1565

### Changes

- Removed all custom deserialization code thanks to #1537, #1541, and #1543.
- The format of the customer info response is now very concise and only in one place (`CustomerInfoResponse`) instead of being passed through dictionaries and magic strings everywhere.
- `CustomerInfo` was combining the dictionaries inside of `"subscriptions"` and `"non_subscriptions"`, despite them having different formats. This is now made explicit through type conversions between the two, (see `CustomerInfoResponse.allTransactionsByProductId`).
- Removed all custom `CustomerInfo` errors since deserialization is automatic, and the underlying `Error` information will be provided thanks to `ErrorUtils.logDecodingError`.
- `CustomerInfo` deserialization always wraps errors into `ErrorCode.customerInfoError`, which is now also covered in a new test.
- Added a new snapshot test that covers `CustomerInfo` serialization by storing the result in a JSON file.

### Other changes:
- `CustomerInfo` equality is now automatic through the `CustomerInfoResponse` `Equatable` conformance.
- Simplified `schemaVersion` handling
- All tests still use `CustomerInfo(data: [String: Any])`, but that method is only visible for tests now, and it uses the underlying `Decodable` implementation (see `CustomerInfo+TestExtensions.swift`).
- Added extra tests for `CustomerInfoResponse` that use a fixture `CustomerInfo.json`, which is easier to maintain than having JSON in code.
- `RawDataContainer` implementation is changed in #1565.

### TODO:
- [x] Fix all tests
- [x] Update `RawValueContainer`
- [x] Consider recovering `CustomerInfoError`
- [x] Look at all previously logged errors
- [x] Finish `CustomerInfoResponseTests`
- [x] Handle all TODOs
- [x] Make `RawDataContainer` actually store all the original data: #1565
@joshdholtz joshdholtz mentioned this pull request May 26, 2022
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.

2 participants