Skip to content

fix some issues #216

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

Closed
wants to merge 5 commits into from
Closed

fix some issues #216

wants to merge 5 commits into from

Conversation

zenz
Copy link
Contributor

@zenz zenz commented Jul 10, 2019

fix liveQuery.subscribe issue.
fix installation issue.

It's now working with my livequery sample.

Copy link
Member

@phillwiggins phillwiggins left a comment

Choose a reason for hiding this comment

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

Please can you check my comments before commit? There's a few things that look confusing.

@@ -64,7 +64,7 @@ class _MyAppState extends State<MyApp> {
Future<void> initData() async {
// Initialize repository
await initRepository();
final CoreStore coreStore = await initCoreStore();
// final CoreStore coreStore = await initCoreStore();
Copy link
Member

Choose a reason for hiding this comment

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

Why has this been removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bwcause it is not used yet. so I comment it out.

Copy link
Member

Choose a reason for hiding this comment

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

This seems like a big and will need to be investigated further.

@@ -100,6 +102,7 @@ abstract class ParseBase {
map.remove(keyVarUpdatedAt);
map.remove(keyVarClassName);
//map.remove(keyVarAcl);
map.remove(keyVarObjectId);
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure this is correct for all calls? Seems strange that it's there in the first place. Have you ran tests that everything else works with this removed? Rather than just installation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do all the test in example. yes.

Copy link
Member

Choose a reason for hiding this comment

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

So this works when saving?

I'm not sure saving an object would work anymore. It will create a new object.

Copy link
Contributor

Choose a reason for hiding this comment

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

@phillwiggins
I'm seeing that they removed the ACL from ParseObject, the code was commented out. When was it done? I've added this functionality to lib and am using this in version 21 on a large project and have not had problem.

@phillwiggins
Copy link
Member

Apologies, I've just noticed as well, we are ok branch release/1.0.23 now.

Please can you make a PR there instead.

@zenz
Copy link
Contributor Author

zenz commented Jul 10, 2019

ok, I will check it tomorrow. it's time to sleep here.

@phillwiggins
Copy link
Member

phillwiggins commented Jul 10, 2019 via email

@RodrigoSMarques
Copy link
Contributor

@phillwiggins
I can not use version 22. It did not pass my test app. Once the merge is done in version 23, I'm going to retake the tests and would like to know why the ACL was removed.

@zenz
Copy link
Contributor Author

zenz commented Jul 11, 2019

@RodrigoSMarques
Please try this, it's based on 1.0.23:

parse_server_sdk:
   git:
     url: [email protected]:zenz/flutter_parse_sdk.git
     ref: 63da5b4243d213824541a35d71c72e82f57c6ac6

And here's the samples to test this SDK v 1.0.23 with livequery.
It just modified from sdk example code to make sure how it works.
https://github.com/zenz/flutter_parse_sample

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.

3 participants