Support user subclass before save #28

acinader · 2016-06-20T19:22:29Z

fixes #25

unit test coverage for _User class

PS the third commit 75c99d2 just adds semicolons. was bothering me and i figured out linting....

acinader · 2016-06-20T19:49:32Z

src/parse-mockdb.js

@@ -112,8 +112,7 @@ function runHook(className, hookType, data) {
      object: model,
      user: "ParseMockDB doesn't define request.user."
    };
-
-    return hook(beforeSaveOrBeforeDeleteRequestObject).bind(model)().then((beforeSaveOverrideValue) => {
+    return hook(beforeSaveOrBeforeDeleteRequestObject).then((beforeSaveOverrideValue) => {


i wish i understood what the .bind(model)() was supposed to do and if it should be there. but now the unit tests pass.

:( any insight you guys can give me as to what it was supposed to do, ideally in the form of a failing test so i can try to figure it out.....

acinader · 2016-06-29T01:07:37Z

@fatuhoku take a look. I rebased your changes, and i took a look at what you did in handle_request, but when i factored it out, i found that it is exactly the same as what was there before?

I fixed the unit tests, but I now need a unit test that fails, cause this really looks like a noop to me? maybe something was fixed in between and when i rebased it, everything just worked??

acinader · 2016-06-29T20:56:44Z

@TylerBrock I think that this can be closed unless you think that the extra unit tests are worth keeping? If so, I'll rebase.

acinader · 2016-06-29T20:57:29Z

@fatuhoku I hope you'll try head again and if there is any functionality that you've got implemented that isn't in head, I'd be glad to work with you to get it worked into head!!!

TylerBrock · 2016-06-29T21:09:41Z

@fatuhoku agree completely with @acinader thank you for all the hard work. Please continue to contribute.

@acinader what is your opinion on keeping the tests? worth it?

Pass in the request object into the beforeSave handler. Rename variable. Rename request. Add failing test for beforeDelete. Fix test by switching the flag. Fail test again by not defining a return value. Reason about beforeSave override value. Fix test by performing deletion. Simplify creation of Brand. Remove TODO by adding assertion for empty collection. Add assertions for the state of the mock collection. Fix semicolons. Replace => syntax with function() syntax. Inline variable. Remove whitespace.

1. don't use switch 2. get unit tests in working order 3. remove code duplication in unit tests.

acinader · 2016-06-29T22:24:39Z

@TylerBrock probably?

npm test | grep ✓ | awk '{print substr($0, index($0, $3))}' | sort | uniq -c

2 rejects the delete if there is a problem
2 rejects the save if there is a problem
2 runs the hook before deleting the object
2 runs the hook before saving the model and persists the object
....

it's certainly valid that those tests should be run twice, once for a "plain" parse class, and once for the special parse class _User, but the code shouldn't be duplicated. I'll see if can quickly refactor, if so I'll amend this. if not, we should probably merge it anyway :).

acinader · 2016-06-29T22:25:42Z

combined the tests for parse objects and _User using @fatuhoku methods. Nice!

yup, @TylerBrock I'd like you to review it for merge.

I figured out how to set up lint for this stuff using eslint.

acinader · 2016-07-02T17:14:59Z

@TylerBrock nudge

TylerBrock · 2016-07-03T16:28:20Z

Hey sorry for the delay. I'm out of town this weekend. Most likely I'll review on Tuesday.

TylerBrock · 2016-07-05T19:20:39Z

I don't think this is mergeable if the deep save test fails. Can we figure out what is going on there?

acinader · 2016-07-05T19:47:45Z

i didn't touch that test https://github.com/HustleInc/parse-mockdb/blob/master/test/test.js#L830

it fails on master too. the only thing i did was change the test from being commented out to marked skip using xit.

which is nice because it shows that the test is skipped when you npm test:

    ✓ should skip and limit items appropriately
    - should deep save and update nested objects
    ✓ successfully uses containsAll query

The test's comment cites an issue that you had opened: parse-community/Parse-SDK-JS#91

the issue was marked resolved in parse sdk in version 1.6.14. I'm running 1.8.5 as our package.json has >=1.6.0

I can open an issue on this repo and look into why it still fails, any background you have might be helpful?

TylerBrock · 2016-07-05T19:56:27Z

Oh, duh, thank you! Much better! It was too early to be looking at diffs apparently.

Yeah, it would be great to get an issue to track it. Would be happy to fill in info where necessary.

TylerBrock · 2016-07-05T21:11:26Z

Two questions:

want me to do a release for this change?
should we update the readme to reflect the current state of the project's support for parse features?

acinader · 2016-07-07T17:13:21Z

Yeah the readme and some code comments need updating. Will do.

acinader · 2016-07-11T16:35:35Z

no opinion on release.

here's an update for the readme: #41

acinader reviewed Jun 20, 2016
View reviewed changes

This was referenced Jun 20, 2016

Get fatuhoku pr's merged... #25

Closed

Add beforeDelete #27

Merged

acinader force-pushed the support-user-subclass-beforeSave branch 5 times, most recently from 7dadc87 to 6a3414b Compare June 29, 2016 01:05

acinader force-pushed the support-user-subclass-beforeSave branch 2 times, most recently from 11ca00f to 9598db4 Compare June 29, 2016 01:11

acinader force-pushed the support-user-subclass-beforeSave branch from 9598db4 to 511c1d7 Compare June 29, 2016 21:53

clean up parse user class handeling

789a65f

1. don't use switch 2. get unit tests in working order 3. remove code duplication in unit tests.

acinader force-pushed the support-user-subclass-beforeSave branch from 511c1d7 to 789a65f Compare June 29, 2016 22:23

Add missing semicolons

75c99d2

I figured out how to set up lint for this stuff using eslint.

TylerBrock merged commit 37acae2 into Hustle:master Jul 5, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support user subclass before save #28

Support user subclass before save #28

acinader commented Jun 20, 2016 •

edited

Loading

acinader Jun 20, 2016

acinader commented Jun 29, 2016

acinader commented Jun 29, 2016

acinader commented Jun 29, 2016

TylerBrock commented Jun 29, 2016

acinader commented Jun 29, 2016

acinader commented Jun 29, 2016

acinader commented Jul 2, 2016

TylerBrock commented Jul 3, 2016

TylerBrock commented Jul 5, 2016

acinader commented Jul 5, 2016

TylerBrock commented Jul 5, 2016

TylerBrock commented Jul 5, 2016 •

edited

Loading

acinader commented Jul 7, 2016

acinader commented Jul 11, 2016

Support user subclass before save #28

Support user subclass before save #28

Conversation

acinader commented Jun 20, 2016 • edited Loading

acinader Jun 20, 2016

Choose a reason for hiding this comment

acinader commented Jun 29, 2016

acinader commented Jun 29, 2016

acinader commented Jun 29, 2016

TylerBrock commented Jun 29, 2016

acinader commented Jun 29, 2016

acinader commented Jun 29, 2016

acinader commented Jul 2, 2016

TylerBrock commented Jul 3, 2016

TylerBrock commented Jul 5, 2016

acinader commented Jul 5, 2016

TylerBrock commented Jul 5, 2016

TylerBrock commented Jul 5, 2016 • edited Loading

acinader commented Jul 7, 2016

acinader commented Jul 11, 2016

acinader commented Jun 20, 2016 •

edited

Loading

TylerBrock commented Jul 5, 2016 •

edited

Loading