batchSet and transactionSet should rely on castedReference like set

[gmarizy]

Fixes #37

In generated code, relying on batch.set(reference, json, options); for batchSet implementation is wrong because we pass a DocumentReference<T> with T a mapping class as a reference with a json data. Cloud_firestore then try to apply the converter but data is already in json. See write_batch.dart from cloud_firestore, line 56.

batch.set(reference, json, options);

Should be replaced with :

    final castedReference = reference.withConverter<Map<String, dynamic>>(
      fromFirestore: (snapshot, options) => throw UnimplementedError(),
      toFirestore: (value, options) => value,
    );

    batch.set(castedReference, json, options);

Like it's done for set method. Same for transaction.

• [ ] Tests pass
• [ ] Appropriate changes to README are included in PR

[rrousselGit]

Thanks! Could you add tests for those?

[gmarizy]

Batch and transaction set are already covered in example/integration_test/document_reference_test.dart. I wanted to run those tests before and after fix for validation, but didn't succeed running them.

[gmarizy]

I should have mentioned that if I couldn't run the test suit, the proposed fix work fine on real projects.

[andreasmpet]

Any progress on this? Having this exact issue now where I wanted to use batchSet and it wouldn't work.

[gmarizy]

Still waiting review