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

Pass context in beforeDelete, afterDelete, beforeFind and Parse.Cloud.run. #6666

Merged
merged 16 commits into from
Jul 10, 2020
Merged

Pass context in beforeDelete, afterDelete, beforeFind and Parse.Cloud.run. #6666

merged 16 commits into from
Jul 10, 2020

Conversation

yog27ray
Copy link
Contributor

@yog27ray yog27ray commented May 3, 2020

This extension the of the #6626. To pass context in following calls.

  1. beforeDelete
  2. afterDelete
  3. beforeFind
  4. Cloud Function

Don't merge until

@mtrezza
Copy link
Member

mtrezza commented May 31, 2020

@yog27ray Do you need any help with the failing cases?

@yog27ray
Copy link
Contributor Author

@mtrezza these failing test cases depends on parse-community/Parse-SDK-JS#1159

@mtrezza
Copy link
Member

mtrezza commented Jun 20, 2020

Got it. Amazing work to see the context now expanded to all these methods! 👍

I should mention some pitfalls I came across with the original PR for save, namely the different routes that need to be considered. Maybe that is also relevant for this PR, see the fix #6736.

@dplewis
Copy link
Member

dplewis commented Jun 20, 2020

Can you guys tag team this issue here #6736

Sorry I’m kinda out of the loop which is why I didn’t merge the SDK PR

@mtrezza
Copy link
Member

mtrezza commented Jun 20, 2020

@dplewis Issue #6736 should be done, it only needs merging of the 2 related PRs. I think after that it makes sense to merge this PR, as it’s an extension of #6736 which is an extension of #6459 as I understand it.

@marcinjakubowski
Copy link

any idea when these context changes may be merged?

 into add-context-to-pending-hooks

� Conflicts:
�	spec/CloudCode.spec.js
@yog27ray
Copy link
Contributor Author

yog27ray commented Jul 4, 2020

@dplewis on travis failing test cases are passing in my local. Can you help me how can I replicate the same.

spec/CloudCode.spec.js Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 4, 2020

Codecov Report

Merging #6666 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6666      +/-   ##
==========================================
- Coverage   93.90%   93.89%   -0.01%     
==========================================
  Files         169      169              
  Lines       12048    12047       -1     
==========================================
- Hits        11314    11312       -2     
- Misses        734      735       +1     
Impacted Files Coverage Δ
src/GraphQL/helpers/objectsQueries.js 90.83% <ø> (ø)
src/GraphQL/loaders/usersQueries.js 97.50% <ø> (ø)
src/ParseServerRESTController.js 98.18% <ø> (ø)
src/Routers/AggregateRouter.js 100.00% <ø> (ø)
src/Routers/AudiencesRouter.js 100.00% <ø> (ø)
src/Routers/ClassesRouter.js 97.93% <ø> (ø)
src/Routers/CloudCodeRouter.js 100.00% <ø> (ø)
src/Routers/FunctionsRouter.js 94.87% <ø> (ø)
src/Routers/IAPValidationRouter.js 85.00% <ø> (ø)
src/Routers/InstallationsRouter.js 83.33% <ø> (ø)
... 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 f095dff...8fde167. Read the comment docs.

@mtrezza
Copy link
Member

mtrezza commented Jul 4, 2020

on travis failing test cases are passing in my local. Can you help me how can I replicate the same.

These seem to be randomly failing tests, unrelated to your PR, see #6758. I restarted the build and the tests pass.

spec/CloudCode.spec.js Outdated Show resolved Hide resolved
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.

Looks good, thanks for removing the lint changes, much easier to read now. Just some minor comments, can you please take another look?

spec/CloudCode.spec.js Outdated Show resolved Hide resolved
spec/CloudCode.spec.js Outdated Show resolved Hide resolved
spec/ParseACL.spec.js Outdated Show resolved Hide resolved
src/GraphQL/helpers/objectsMutations.js Show resolved Hide resolved
src/Routers/ClassesRouter.js Show resolved Hide resolved
@yog27ray
Copy link
Contributor Author

yog27ray commented Jul 8, 2020

@mtrezza can you restart the build.

@mtrezza
Copy link
Member

mtrezza commented Jul 8, 2020

Done. You can also restart the build by pushing an empty commit git commit --allow-empty -m "Trigger Travis-CI"

Copy link
Member

@dplewis dplewis left a comment

Choose a reason for hiding this comment

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

LGTM!

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