feat: Add trigger `beforeUnsubscribe` for LiveQuery by sadortun · Pull Request #7419 · parse-community/parse-server

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

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.

mbfakourii · 2025-03-17T06:46:01Z

@mtrezza

Any progress on this PR?

mtrezza · 2025-03-17T18:52:08Z

@mbfakourii I don't have write access to this PR, so the PR author @sadortun would need to merge the base branch for the CI to run. Alternatively, someone could use this branch and open their own PR to continue make it ready for the CI.

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

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

Uh oh!

Conversation

sadortun commented Jun 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

New Pull Request Checklist

Issue Description

Approach

TODOs before merging

Uh oh!

mtrezza commented Jun 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sadortun commented Jun 6, 2021

Uh oh!

mtrezza left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtrezza left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jun 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sadortun commented Jun 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrezza commented Jun 7, 2021

Uh oh!

sadortun commented Jun 7, 2021

Uh oh!

sadortun commented Jun 7, 2021

Uh oh!

Uh oh!

mtrezza commented Jun 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dplewis left a comment

Choose a reason for hiding this comment

Uh oh!

mtrezza left a comment

Choose a reason for hiding this comment

Uh oh!

mtrezza commented Jun 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sadortun commented Jun 22, 2021

Uh oh!

mtrezza commented Jun 22, 2021

Uh oh!

mtrezza commented Jun 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrezza commented Jul 25, 2021

Uh oh!

sadortun commented Jul 25, 2021

Uh oh!

mtrezza commented Jul 25, 2021

Uh oh!

mtrezza commented Jul 29, 2021

Uh oh!

sadortun commented Aug 11, 2021

Uh oh!

sadortun commented Sep 3, 2021

Uh oh!

mtrezza left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dblythy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dblythy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dblythy Sep 4, 2021

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

mtrezza commented Mar 17, 2025 •

edited

Loading