-
Notifications
You must be signed in to change notification settings - Fork 181
Update Flutter SDK to add offline support #618
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
Conversation
00e5b11
to
6358c99
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Let's make sure the generated SDK looks good as well once we have changes to the API and swagger spec with updated requirement. We must update the mock endpoints to support these new functionalities as well.
02fb129
to
9e7e0c4
Compare
@lohanidamodar, what were you thinking we would do for the mock endpoints? |
Can we have mock endpoints with cache labels, and may be run test and check if data is cached? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Left few comments. Let's also see the generated SDK with new Spec and this branch 🙏🏻
src/Spec/Swagger2.php
Outdated
'offline' => [ | ||
'model' => $method['x-appwrite']['offline-model'] ?? '', | ||
'key' => $method['x-appwrite']['offline-key'] ?? '', | ||
'model' => $method['x-appwrite']['offline-model'] ?? '', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'model' => $method['x-appwrite']['offline-model'] ?? '', |
Looks like this was defined twice
tests/languages/dart/tests.dart
Outdated
@@ -114,7 +114,7 @@ void main() async { | |||
print(Permission.update(Role.user(ID.custom('userid'), 'unverified'))); | |||
|
|||
// ID helper tests | |||
print(ID.unique()); | |||
// print(ID.unique()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can remove this or we can test this by printing twice making sure it gives back unique ID each time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great idea!
tests/languages/flutter/tests.dart
Outdated
@@ -147,7 +147,7 @@ void main() async { | |||
print(Permission.update(Role.user(ID.custom('userid'), 'unverified'))); | |||
|
|||
// ID helper tests | |||
print(ID.unique()); | |||
// print(ID.unique()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above.
603ae18
to
b37f539
Compare
@lohanidamodar, I've updated the PR based on your feedback including adding the mock labels: appwrite/appwrite#5160 |
3a1a9e2
to
50950e3
Compare
50950e3
to
d651cec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just some hard-coded Appwrite
references to template 👍
d651cec
to
3aa4cf3
Compare
What does this PR do?
Update the Flutter SDK to add Offline Support.
Test Plan
Manual
Related PRs and Issues
TODO
Have you read the Contributing Guidelines on issues?
Yes