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

Before Connect + Before Subscribe help required #6793

Merged
merged 5 commits into from
Jul 17, 2020

Conversation

dblythy
Copy link
Member

@dblythy dblythy commented Jul 13, 2020

Hey guys,

This is my second attempt at creating two new cloud code triggers which affect LiveQuery: beforeConnect and beforeSubscribe.

beforeConnect allows validation prior to a LiveQuery opening connection. This is not class specific.

Parse.Cloud.beforeConnect(request => {
      if (!request.user) {
          throw "Please login before you attempt to connect."
      }
  });

beforeSubscribe handles the .subscribe methods from LiveQuery. Can be used to validate users, or to mutate the request.query, enforcing fields, equalTo, or whatever required.

Parse.Cloud.beforeSubscribe(Parse.User, request => {
    if (!request.user) {
        throw "Please login before you attempt to connect."
    }
    let query = request.query; // the Parse.Query
    query.select("name","year")
});

However, as I'm a JS noob, I'm struggling to create tests that will pass. I was wondering if someone would be kind to help me close this out (I'll buy you a box of beers because I've been pulling my hair out trying to work this out).

Also my linter changed some other things, couldn't work out how to stop it (again, I'm a VS noob too).

Thank you all for your hard work!!

@dblythy
Copy link
Member Author

dblythy commented Jul 13, 2020

In relation to #6649, #6642, and #6779.

@dblythy
Copy link
Member Author

dblythy commented Jul 13, 2020

Here are the failing tests

@dplewis
Copy link
Member

dplewis commented Jul 13, 2020

@dblythy Can you give me access to your master branch? It's best practice to create a separate branch for every PR.

@dblythy
Copy link
Member Author

dblythy commented Jul 14, 2020

Thanks for the tip @dplewis. I've invited you to my branch.

@dplewis
Copy link
Member

dplewis commented Jul 14, 2020

Got it. I’ll review in the AM.

@dplewis
Copy link
Member

dplewis commented Jul 15, 2020

@dblythy I need to add a few more tests. Is there anything else that should be returned in beforeConnect and beforeSubscribe request handler?

@dplewis
Copy link
Member

dplewis commented Jul 15, 2020

@dblythy This beforeSubscribe will need a tiny update on the JS SDK to properly handle errors

@dblythy
Copy link
Member Author

dblythy commented Jul 16, 2020

@dplewis thank you so much for your help again. Is it worth creating a test to make sure modifying req.query affects the subscription?

@dplewis
Copy link
Member

dplewis commented Jul 16, 2020

No problem, add as many tests as you need.

@dblythy
Copy link
Member Author

dblythy commented Jul 16, 2020

Ok, no worries. So once the JS SDK is updated, the tests should pass and we can submit the PR? 😊

@codecov
Copy link

codecov bot commented Jul 16, 2020

Codecov Report

Merging #6793 into master will decrease coverage by 0.06%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6793      +/-   ##
==========================================
- Coverage   93.93%   93.87%   -0.07%     
==========================================
  Files         169      169              
  Lines       12054    12197     +143     
==========================================
+ Hits        11323    11450     +127     
- Misses        731      747      +16     
Impacted Files Coverage Δ
src/LiveQuery/Client.js 100.00% <ø> (ø)
src/triggers.js 92.09% <83.87%> (-1.09%) ⬇️
src/LiveQuery/ParseLiveQueryServer.js 94.56% <100.00%> (+0.15%) ⬆️
src/cloud-code/Parse.Cloud.js 98.52% <100.00%> (+0.11%) ⬆️
src/Adapters/Auth/google.js 92.75% <0.00%> (-7.25%) ⬇️
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 92.87% <0.00%> (-0.66%) ⬇️
src/RestWrite.js 93.64% <0.00%> (-0.33%) ⬇️
src/Controllers/DatabaseController.js 95.28% <0.00%> (-0.07%) ⬇️
src/rest.js 98.86% <0.00%> (ø)
src/Options/Definitions.js 100.00% <0.00%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1cc3211...97c08b2. Read the comment docs.

@dplewis dplewis marked this pull request as ready for review July 17, 2020 00:07
Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

LGTM

@dplewis dplewis merged commit 44015c3 into parse-community:master Jul 17, 2020
@jadsonlourenco
Copy link

Maybe we can add the trigger "afterDisconnect" too?

@dblythy
Copy link
Member Author

dblythy commented Jul 28, 2020

@jadsonlourenco I believe you can use onLiveQueryEvent for running a function after a disconnect.

@jadsonlourenco
Copy link

@jadsonlourenco I believe you can use onLiveQueryEvent for running a function after a disconnect.

Yes, I'm using, but like this PR did, if we have a trigger to handle before connect on a "live" system is good to know when the client disconnect, using it as Parse api will be more easy to implement, just to make the Parse api more complete, I think.

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.

4 participants