RevenueCat / firestore-revenuecat-purchases

Apache License 2.0
23 stars 7 forks source link

Remove redundant write to document #39

Closed dennismuench closed 2 years ago

dennismuench commented 2 years ago

Remove update(payloadToWrite) after set(payloadToWrite, { merge: true }) as this would just overwrite the document directly after creating or merging it with the exact same data. This is probably just an accident. If you want to ensure the document gets either created or replaced, use set() without the merge flag. If you want to ensure the document gets written, wrap it in a transaction. I just stumbled upon this, thanks for the extension.

dennismuench commented 2 years ago

Of course I might have overlooked something, in which case I am sorry to have bothered you guys.

alfondotnet commented 2 years ago

Hi @dennismuench , no worries!

The reason why there is an update right after the merge set is because in the case where you want to merge all new top-level keys in, but completely overwriting the values of these, I didn't find a way around that other than this.

Use case is when an entitlement has been removed, set with merge: false would overwrite pre-existent keys (customers might want to store other things in there!).

See this test: https://github.com/RevenueCat/firestore-revenuecat-purchases/blob/28870aeaa5167d5d6ea18dd7cebae2e4de59e42e/functions/src/__tests__/events.test.ts#L222 And: https://github.com/RevenueCat/firestore-revenuecat-purchases/blob/28870aeaa5167d5d6ea18dd7cebae2e4de59e42e/functions/src/__tests__/events.test.ts#L303

I'll be closing this off for now but feel free to re-open if you know of a better way of doing this!