feat: Add trigger `beforeUnsubscribe` for LiveQuery #7419

sadortun · 2021-06-06T02:45:15Z

New Pull Request Checklist

I am not disclosing a vulnerability.
I am creating this PR in reference to an issue.

Issue Description

There is a very useful Before Subscribe Trigger, this PR add its counterpart beforeUnsubscribe

Related issue: #7420

Approach

TODOs before merging

Add test cases
Add entry to changelog

mtrezza · 2021-06-06T07:14:20Z

Thanks for opening this PR.

Please make sure to also open an issue related to this PR.

sadortun · 2021-06-06T16:11:41Z

@mtrezza done.

mtrezza

Approved too quickly...

mtrezza

Just needs a lint-fix it seems.
And can you add a test for this new trigger?

codecov · 2021-06-06T16:22:15Z

Codecov Report

Patch coverage: 88.88% and project coverage change: -0.01% ⚠️

Comparison is base (3d6d50e) 94.31% compared to head (5d07966) 94.31%.
Report is 1 commits behind head on alpha.

❗ Current head 5d07966 differs from pull request most recent head 1d1660a. Consider uploading reports for the commit 1d1660a to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##            alpha    #7419      +/-   ##
==========================================
- Coverage   94.31%   94.31%   -0.01%     
==========================================
  Files         186      186              
  Lines       14770    14786      +16     
==========================================
+ Hits        13931    13946      +15     
- Misses        839      840       +1

Files Changed	Coverage Δ
src/triggers.js	`95.35% <ø> (ø)`
src/LiveQuery/ParseLiveQueryServer.js	`95.72% <85.71%> (-0.32%)`	⬇️
src/cloud-code/Parse.Cloud.js	`99.31% <100.00%> (+0.01%)`	⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sadortun · 2021-06-06T19:13:22Z

@mtrezza I had to commit with --no-verify, the prettier doesn't seems to work, it does with en error saying there Is no files in /src**/

I'll try to have a look later

mtrezza · 2021-06-07T09:18:30Z

If you run npm run prettier it should work. But if that causes a lot of changes in files that are unrelated to this PR, please only run it for the files you edited.
Then run npm run lint-fix.

sadortun · 2021-06-07T13:26:17Z

@mtrezza Sadly prettier wont work with the npm run (im using windows)

This fixes the issue. Do you want a PR ? Should i commit this in this PR ?

- prettier --write '{src,spec}/{**/*,*}.js'
+ prettier --write {src,spec}/{**/*,*}.js

sadortun · 2021-06-07T15:09:22Z

@mtrezza Docs added parse-community/docs#833

src/cloud-code/Parse.Cloud.js

mtrezza · 2021-06-07T16:43:54Z

This fixes the issue. Do you want a PR ? Should i commit this in this PR ?

I assume that it still works on unix then, yes please go ahead, I'll test it.

dplewis

LGTM! @mtrezza anything you want to add?

mtrezza

Looks good. Can you please update the ToDo list and check if we have things like changelog entry.

mtrezza · 2021-06-21T22:21:13Z

To clarify the TODO list, I think we can delete these, because they are irrelevant in this PR:

Add security check
Add new Parse Error codes to Parse JS SDK
Add changes to documentation (guides, repository pages, in-code descriptions)
...

This PR just needs this I think, then we can do a final review:

Add entry to changelog

sadortun · 2021-06-22T02:08:29Z

@mtrezza Changelog entry added

mtrezza · 2021-06-22T05:25:46Z

@sadortun I removed the n/a TODOs from the post. To clarify, TODOs that do not apply to a PR are removed, not checked, otherwise a reviewer may think you actually made these changes.

mtrezza · 2021-06-26T23:43:34Z

Can you take a look at the coverage for src/LiveQuery/ParseLiveQueryServer.js#L848? https://github.com/parse-community/parse-server/pull/7419/checks?check_run_id=2881789028

mtrezza · 2021-07-25T11:52:49Z

@sadortun Would you mind rebasing this on master and make sure the coverage is fine? I think this will be good to merge then.

sadortun · 2021-07-25T16:25:35Z

Ofc, sorry about the delayS, on this and others PRs !!

mtrezza · 2021-07-25T17:23:55Z

No worries, thanks for looking into this.

mtrezza · 2021-07-29T13:26:37Z

I have removed myself from review for now. Please feel free to request again when the merge conflicts are resolved.

sadortun · 2021-08-11T17:56:22Z

@mtrezza
✅ Rebased
✅ Fixed coverage

mtrezza · 2021-09-03T00:56:17Z

⚠️ Important change for merging PRs from Parse Server 5.0 onwards!

We are planning to release the first beta version of Parse Server 5.0 in October 2021.

If a PR contains a breaking change and is not merged before the beta release of Parse Server 5.0, it cannot be merged until the end of 2022. Instead it has to follow the Deprecation Policy and phase-in breaking changes to be merged during the course of 2022.

One of the most voiced community feedbacks was the demand for predictability in breaking changes to make it easy to upgrade Parse Server. We have made a first step towards this by introducing the Deprecation Policy in February 2021 that assists to phase-in breaking changes, giving developers time to adapt. We will follow-up with the introduction of Release Automation and a branch model that will allow breaking changes only with a new major release, scheduled for the beginning of each calendar year.

We understand that some PRs are a long time in the making and we very much appreciate your contribution. We want to make it easy for PRs that contain a breaking change and were created before the introduction of the Deprecation Policy. These PRs can be merged with a breaking change without being phased-in before the beta release of Parse Server 5.0. We are making this exception because we appreciate that this is a time of transition that requires additional effort from contributors to adapt. We encourage everyone to prepare their PRs until the end of September and account for review time and possible adaptions.

If a PR contains a breaking change and should be merged before the beta release, please mention @parse-community/server-maintenance and we will coordinate with you to merge the PR.

Thanks for your contribution and support during this transition to Parse Server release automation!

sadortun · 2021-09-03T16:34:32Z

@mtrezza @dblythy

✅ Fixed last discussions
✅ Rebased on master

mtrezza

Looks amazing to me!

Let's see what @dblythy says, nothing escapes his eyes usually.

dblythy

Looks really good! Only a minor nit

src/LiveQuery/ParseLiveQueryServer.js

dblythy

Nice job @sadortun! Sorry I didn't realise that there were other hard coded errors in the file and you were just copying the formatting. We should fix that in a different PR

Actually, a few other things:

dblythy · 2021-09-04T04:01:03Z

src/LiveQuery/ParseLiveQueryServer.js

+    const className = subscription.className;
+    const trigger = getTrigger(className, 'beforeUnsubscribe', Parse.applicationId);
+    if (trigger) {
+      const auth = await this.getAuthFromClient(client, request.requestId, request.sessionToken);


In the other uses, getAuthFromClient is wrapped in the try / catch block. I don't think it throws, but could you add it to the try / catch, or add a test case for beforeUnsubscribe without the user.signUp?

@sadortun Could you address this comment, then I believe this should be good to merge.

src/cloud-code/Parse.Cloud.js

mtrezza · 2022-02-12T18:32:30Z

@sadortun Do you think we could get this ready for merge?

sadortun · 2022-02-12T19:49:28Z

Ah, sorry, i was sure it was already merged !

I'll have a look later ~~today~~.

Edit: sorry about the delay(s) I haven't forgotten you!

parse-github-assistant · 2022-02-12T23:02:36Z

Thanks for opening this pull request!

🎉 We are excited about your hands-on contribution!

mtrezza · 2022-05-07T19:18:21Z

@sadortun came across this PR; it looks almost ready to merge; would you want to take another look at the open comments, so we can merge this?

parse-github-assistant · 2023-08-13T13:54:15Z

I will reformat the title to use the proper commit message syntax.

sadortun force-pushed the pr_beforeUnsubscribe branch from 21034a7 to 2e70b92 Compare June 6, 2021 02:47

mtrezza approved these changes Jun 6, 2021

View reviewed changes

mtrezza requested changes Jun 6, 2021

View reviewed changes

sadortun force-pushed the pr_beforeUnsubscribe branch from c480f3d to 1ae5c56 Compare June 7, 2021 14:25

sadortun mentioned this pull request Jun 7, 2021

Add beforeUnsubscribe trigger doc parse-community/docs#833

Open

dplewis suggested changes Jun 7, 2021

View reviewed changes

src/cloud-code/Parse.Cloud.js Outdated Show resolved Hide resolved

dplewis previously approved these changes Jun 8, 2021

View reviewed changes

mtrezza requested changes Jun 8, 2021

View reviewed changes

sadortun requested a review from mtrezza June 25, 2021 12:19

mtrezza removed their request for review July 29, 2021 13:25

sadortun force-pushed the pr_beforeUnsubscribe branch from 82998d2 to 1ed8d05 Compare August 11, 2021 17:55

sadortun requested a review from mtrezza August 11, 2021 17:56

sadortun mentioned this pull request Aug 12, 2021

chore(ci): Remove prettier quoted argument to fix issue on Windows #7489

Merged

2 tasks

sadortun force-pushed the pr_beforeUnsubscribe branch from f53bdca to 4bcaf93 Compare September 3, 2021 16:33

mtrezza previously approved these changes Sep 3, 2021

View reviewed changes

dblythy suggested changes Sep 3, 2021

View reviewed changes

src/LiveQuery/ParseLiveQueryServer.js Outdated Show resolved Hide resolved

dblythy approved these changes Sep 4, 2021

View reviewed changes

dblythy reviewed Sep 4, 2021

View reviewed changes

mtrezza changed the title ~~[Feature] Add a Trigger for BeforeUnsubscribe on live queries~~ Add a Trigger for BeforeUnsubscribe on live queries Feb 12, 2022

mtrezza changed the title ~~Add a Trigger for BeforeUnsubscribe on live queries~~ feat: add trigger beforeUnsubscribe for LiveQuery Feb 12, 2022

mtrezza dismissed stale reviews from dplewis and themself via ab796c2 April 3, 2022 23:40

mtrezza force-pushed the alpha branch from 59215e6 to e6d7d8f Compare May 1, 2022 02:29

sadortun added 7 commits August 13, 2023 09:39

Added beforeUnsubscribe Trigger

41daddb

Remove duplicate onLiveQueryEvent

98d48de

Add logged-in user to add missing coverage

7bb0d2d

Updated CHANGELOG

157f452

Add username check and try/catch block

927ca63

Update ParseLiveQueryServer.js

55be103

Clean up after rebase

d2c37b0

sadortun force-pushed the pr_beforeUnsubscribe branch from ab796c2 to d2c37b0 Compare August 13, 2023 13:54

parse-github-assistant bot changed the title ~~feat: add trigger beforeUnsubscribe for LiveQuery~~ feat: Add trigger beforeUnsubscribe for LiveQuery Aug 13, 2023

sadortun requested review from dplewis and mtrezza September 24, 2023 17:28

Merge branch 'alpha' into pr_beforeUnsubscribe

1d1660a

mtrezza requested a review from a team September 26, 2023 19:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add trigger `beforeUnsubscribe` for LiveQuery #7419

feat: Add trigger `beforeUnsubscribe` for LiveQuery #7419

sadortun commented Jun 6, 2021 •

edited

Loading

mtrezza commented Jun 6, 2021 •

edited

Loading

sadortun commented Jun 6, 2021

mtrezza left a comment •

edited

Loading

mtrezza left a comment •

edited

Loading

codecov bot commented Jun 6, 2021 •

edited

Loading

sadortun commented Jun 6, 2021 •

edited

Loading

mtrezza commented Jun 7, 2021

sadortun commented Jun 7, 2021

sadortun commented Jun 7, 2021

mtrezza commented Jun 7, 2021 •

edited

Loading

dplewis left a comment

mtrezza left a comment

mtrezza commented Jun 21, 2021 •

edited

Loading

sadortun commented Jun 22, 2021

mtrezza commented Jun 22, 2021

mtrezza commented Jun 26, 2021 •

edited

Loading

mtrezza commented Jul 25, 2021

sadortun commented Jul 25, 2021

mtrezza commented Jul 25, 2021

mtrezza commented Jul 29, 2021

sadortun commented Aug 11, 2021

mtrezza commented Sep 3, 2021

⚠️ Important change for merging PRs from Parse Server 5.0 onwards!

sadortun commented Sep 3, 2021

mtrezza left a comment •

edited

Loading

dblythy left a comment

dblythy left a comment •

edited

Loading

dblythy Sep 4, 2021

mtrezza Oct 3, 2023

mtrezza commented Feb 12, 2022

sadortun commented Feb 12, 2022 •

edited

Loading

parse-github-assistant bot commented Feb 12, 2022 •

edited

Loading

mtrezza commented May 7, 2022 •

edited

Loading

parse-github-assistant bot commented Aug 13, 2023

feat: Add trigger beforeUnsubscribe for LiveQuery #7419

Are you sure you want to change the base?

feat: Add trigger beforeUnsubscribe for LiveQuery #7419

Conversation

sadortun commented Jun 6, 2021 • edited Loading

New Pull Request Checklist

Issue Description

Approach

TODOs before merging

mtrezza commented Jun 6, 2021 • edited Loading

sadortun commented Jun 6, 2021

mtrezza left a comment • edited Loading

Choose a reason for hiding this comment

mtrezza left a comment • edited Loading

Choose a reason for hiding this comment

codecov bot commented Jun 6, 2021 • edited Loading

Codecov Report

sadortun commented Jun 6, 2021 • edited Loading

mtrezza commented Jun 7, 2021

sadortun commented Jun 7, 2021

sadortun commented Jun 7, 2021

mtrezza commented Jun 7, 2021 • edited Loading

dplewis left a comment

Choose a reason for hiding this comment

mtrezza left a comment

Choose a reason for hiding this comment

mtrezza commented Jun 21, 2021 • edited Loading

sadortun commented Jun 22, 2021

mtrezza commented Jun 22, 2021

mtrezza commented Jun 26, 2021 • edited Loading

mtrezza commented Jul 25, 2021

sadortun commented Jul 25, 2021

mtrezza commented Jul 25, 2021

mtrezza commented Jul 29, 2021

sadortun commented Aug 11, 2021

mtrezza commented Sep 3, 2021

⚠️ Important change for merging PRs from Parse Server 5.0 onwards!

sadortun commented Sep 3, 2021

mtrezza left a comment • edited Loading

Choose a reason for hiding this comment

dblythy left a comment

Choose a reason for hiding this comment

dblythy left a comment • edited Loading

Choose a reason for hiding this comment

dblythy Sep 4, 2021

Choose a reason for hiding this comment

mtrezza Oct 3, 2023

Choose a reason for hiding this comment

mtrezza commented Feb 12, 2022

sadortun commented Feb 12, 2022 • edited Loading

parse-github-assistant bot commented Feb 12, 2022 • edited Loading

Thanks for opening this pull request!

mtrezza commented May 7, 2022 • edited Loading

parse-github-assistant bot commented Aug 13, 2023

feat: Add trigger `beforeUnsubscribe` for LiveQuery #7419

feat: Add trigger `beforeUnsubscribe` for LiveQuery #7419

sadortun commented Jun 6, 2021 •

edited

Loading

mtrezza commented Jun 6, 2021 •

edited

Loading

mtrezza left a comment •

edited

Loading

mtrezza left a comment •

edited

Loading

codecov bot commented Jun 6, 2021 •

edited

Loading

sadortun commented Jun 6, 2021 •

edited

Loading

mtrezza commented Jun 7, 2021 •

edited

Loading

mtrezza commented Jun 21, 2021 •

edited

Loading

mtrezza commented Jun 26, 2021 •

edited

Loading

mtrezza left a comment •

edited

Loading

dblythy left a comment •

edited

Loading

sadortun commented Feb 12, 2022 •

edited

Loading

parse-github-assistant bot commented Feb 12, 2022 •

edited

Loading

mtrezza commented May 7, 2022 •

edited

Loading