Allowing field deletes with merge-enabled sets #248
Conversation
| const merge = options.merge !== undefined ? options.merge : false; | ||
| let updateData = ObjectValue.EMPTY; | ||
|
|
||
| objUtils.forEach(input as Dict<AnyJs>, (key, value) => { |
There was a problem hiding this comment.
Why do we need to loop over the top-level keys instead of just calling parseData() directly on the entire object? Did we clean that up in Android and forget to port?
There was a problem hiding this comment.
Yeah, I think we forgot to port this back. Fixed.
| } else { | ||
| // We shouldn't encounter delete sentinels for queries or non-merge set() calls. | ||
| throw context.createError( | ||
| 'FieldValue.delete() can only be used with update() and set() with SetOptions.merge()' |
| @@ -140,6 +140,33 @@ apiDescribe('Database', persistence => { | |||
| }); | |||
| }); | |||
|
|
|||
| asyncIt('can delete field using merge', () => { | |||
| return withTestDb(persistence, db => { | |||
| const doc = db.doc('rooms/Eros'); | |||
There was a problem hiding this comment.
Can you use withTestDoc(persistence, doc => ... ) instead? I have a PR I'm going to send out that removes all these hardcoded collection / documents since they could potentially cause flakiness in travis.
There was a problem hiding this comment.
Sounds good. Done.
Sorry I pushed this just a second after the first commit. You are too fast as always :) |
|
@jshcrowthe Please merge this. All tests are passing locally for me: HeadlessChrome 0.0.0 (Mac OS X 10.12.6): Executed 810 of 823 (skipped 13) SUCCESS (1 min 52.27 secs / 1 min 49.864 secs) |
This adds support for deleting fields with merges.
This is a port of cr/172802651.