Skip to content

Commit

Permalink
fix: fixed createOrUpdateWithAccessor
Browse files Browse the repository at this point in the history
- changed createOrUpdateWithAccessor to only use set(), instead of update(), as update does not call the converters the same way set does. Updated documentation to clarify this and the usage of update.
  • Loading branch information
dereekb committed May 9, 2022
1 parent 2de30e0 commit 243d0d3
Show file tree
Hide file tree
Showing 13 changed files with 80 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { guestbookEntryUpdateEntry } from './guestbookentry.update';
import { GuestbookEntry, UpdateGuestbookEntryParams } from '@dereekb/demo-firebase';
import { demoGuestbookEntryContext, DemoApiFunctionContextFixture, demoApiFunctionContextFactory, demoAuthorizedUserContext, demoGuestbookContext } from '../../../test/fixture';
import { WrappedCloudFunction } from '@dereekb/firebase-server';
import { isDate, isValid } from 'date-fns';

demoApiFunctionContextFactory((f: DemoApiFunctionContextFixture) => {

Expand Down Expand Up @@ -81,11 +82,17 @@ demoApiFunctionContextFactory((f: DemoApiFunctionContextFixture) => {
expect(exists).toBe(true);

data = (await userGuestbookEntry.snapshotData())!;


expect(data).toBeDefined();
expect(data?.message).toBe(newMessage);
expect(data?.signed).toBe(signed);
expect(data?.createdAt).not.toBeFalsy();
expect(data?.updatedAt).not.toBeFalsy();
expect(isDate(data?.createdAt)).toBe(true);
expect(isDate(data?.updatedAt)).toBe(true);
expect(isValid(data?.createdAt)).toBe(true);
expect(isValid(data?.updatedAt)).toBe(true);
});

});
Expand Down
2 changes: 1 addition & 1 deletion apps/demo-firebase/src/lib/guestbook/guestbook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export interface GuestbookEntry extends UserRelatedById {
*/
createdAt: Date;
/**
* Whether or not the entry has been published. This cannot be changed once published.
* Whether or not the entry has been published. It can be unpublished at any time by the user.
*/
published?: boolean;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { MatDialog } from '@angular/material/dialog';
import { HandleActionFunction } from '@dereekb/dbx-core';
import { DemoGuestbookEntryFormValue } from '../../../../shared/modules/guestbook/component/guestbook.entry.form.component';
import { GuestbookEntryDocumentStore } from './../../../../shared/modules/guestbook/store/guestbook.entry.document.store';
import { IsModifiedFunction } from '@dereekb/rxjs';
import { map, of, switchMap } from 'rxjs';

export interface DemoGuestbookEntryPopupComponentConfig {
guestbookEntryDocumentStore: GuestbookEntryDocumentStore;
Expand All @@ -13,8 +15,8 @@ export interface DemoGuestbookEntryPopupComponentConfig {
template: `
<dbx-dialog-content>
<p class="dbx-note">Enter your message for the guest book.</p>
<div dbxAction [dbxActionHandler]="handleUpdateEntry">
<demo-guestbook-entry-form dbxActionForm></demo-guestbook-entry-form>
<div dbxAction dbxActionLogger dbxActionEnforceModified [dbxActionHandler]="handleUpdateEntry">
<demo-guestbook-entry-form dbxActionForm [dbxFormSource]="data$" [dbxActionFormModified]="isFormModified"></demo-guestbook-entry-form>
<p></p>
<dbx-button [raised]="true" [text]="(exists$ | async) ? 'Update Entry' : 'Create Entry'" dbxActionButton></dbx-button>
</div>
Expand All @@ -27,6 +29,10 @@ export class DemoGuestbookEntryPopupComponent extends AbstractDialogDirective<an
return this.data.guestbookEntryDocumentStore;
}

get data$() {
return this.guestbookEntryDocumentStore.data$;
}

get exists$() {
return this.guestbookEntryDocumentStore.exists$;
}
Expand All @@ -37,6 +43,22 @@ export class DemoGuestbookEntryPopupComponent extends AbstractDialogDirective<an
});
}

readonly isFormModified: IsModifiedFunction<DemoGuestbookEntryFormValue> = (value) => {
return this.exists$.pipe(
switchMap((exists) => {
if (exists) {
return this.data$.pipe(
map((current) => {
const isModified = Boolean(current.message !== value.message) || Boolean(current.signed !== value.signed) || Boolean(current.published !== value.published);
return isModified;
}));
} else {
return of(true);
}
})
);
}

readonly handleUpdateEntry: HandleActionFunction = (value: DemoGuestbookEntryFormValue, context) => {
context.startWorkingWithLoadingStateObservable(this.guestbookEntryDocumentStore.updateEntry(value));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { GuestbookEntry } from "@dereekb/demo-firebase";
import { FormlyFieldConfig } from "@ngx-formly/core";
import { guestbookEntryFields } from "./guestbook.entry.form";

export interface DemoGuestbookEntryFormValue extends Pick<GuestbookEntry, 'message' | 'signed'> { }
export interface DemoGuestbookEntryFormValue extends Pick<GuestbookEntry, 'message' | 'signed' | 'published'> { }

@Component({
template: `<dbx-formly></dbx-formly>`,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { textAreaField, textField } from "@dereekb/dbx-form";
import { textAreaField, textField, toggleField } from "@dereekb/dbx-form";
import { GUESTBOOK_ENTRY_MESSAGE_MAX_LENGTH, GUESTBOOK_ENTRY_SIGNED_MAX_LENGTH } from "@dereekb/demo-firebase";

export function guestbookEntryFields() {
return [
guestbookEntryMessageField(),
guestbookEntrySignedField()
guestbookEntrySignedField(),
guestbookEntryPublishedField()
];
}

Expand All @@ -15,3 +16,7 @@ export function guestbookEntryMessageField() {
export function guestbookEntrySignedField() {
return textField({ key: 'signed', label: 'Signed', maxLength: GUESTBOOK_ENTRY_SIGNED_MAX_LENGTH, required: true });
}

export function guestbookEntryPublishedField() {
return toggleField({ key: 'published', label: 'Published' });
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export class DocActionFormComponent {
readonly isFormModified: IsModifiedFunction<DocActionFormExampleValue> = (value: DocActionFormExampleValue) => {
return this.defaultValue$.pipe(
map((defaultValue) => {
const isModified = Boolean(value.name === defaultValue.name) || !isSameMinute(value.date, defaultValue.date);
const isModified = Boolean(value.name !== defaultValue.name) || !isSameMinute(value.date, defaultValue.date);
return isModified;
}));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Directive, Host, Input, OnInit, OnDestroy } from '@angular/core';
import { Maybe } from '@dereekb/util';
import { BehaviorSubject, combineLatest, delay } from 'rxjs';
import { AbstractSubscriptionDirective } from '../../../subscription';
import { DbxActionContextStoreSourceInstance } from '../../action.store.source';
Expand Down Expand Up @@ -32,13 +33,13 @@ export class DbxActionEnforceModifiedDirective extends AbstractSubscriptionDirec
this.source.enable(APP_ACTION_ENFORCE_MODIFIED_DIRECTIVE_KEY);
}

@Input('[dbxActionEnforceModified]')
@Input('dbxActionEnforceModified')
get enabled(): boolean {
return this._enabled.value;
}

set enabled(enabled: boolean) {
this._enabled.next(enabled ?? true);
set enabled(enabled: Maybe<string | boolean>) {
this._enabled.next(Boolean(enabled) ?? true);
}

}
13 changes: 12 additions & 1 deletion packages/dbx-core/src/lib/pipe/date/tojsdate.pipe.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isValid } from 'date-fns';
import { Pipe, PipeTransform } from '@angular/core';
import { toJsDate } from '@dereekb/date';
import { DateOrDateString, Maybe } from '@dereekb/util';
Expand All @@ -6,7 +7,17 @@ import { DateOrDateString, Maybe } from '@dereekb/util';
export class ToJsDatePipe implements PipeTransform {

public static toJsDate(input: Maybe<DateOrDateString>): Maybe<Date> {
return (input) ? toJsDate(input) : undefined;
let date: Maybe<Date>;

if (input != null) {
date = toJsDate(input);

if (!isValid(date)) {
date = undefined;
}
}

return date;
}

transform(input: Maybe<DateOrDateString>): Maybe<Date> {
Expand Down
5 changes: 5 additions & 0 deletions packages/dbx-firebase/src/lib/model/store/store.document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,11 @@ export class AbstractDbxFirebaseDocumentStore<T, D extends FirestoreDocument<T>
shareReplay(1)
);

readonly doesNotExist$: Observable<boolean> = this.exists$.pipe(
map(x => !x),
shareReplay(1)
);

// MARK: State Changes
/**
* Sets the id of the document to load.
Expand Down
14 changes: 9 additions & 5 deletions packages/firebase/src/lib/common/firestore/accessor/accessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ export interface FirestoreDocumentDataAccessor<T> extends DocumentReferenceRef<T
set(data: PartialWithFieldValue<T>, options: SetOptions): Promise<WriteResult | void>;
set(data: WithFieldValue<T>): Promise<WriteResult | void>;
/**
* Updates the data in the database. If the document doesn't exist, it will fail.
* Directly updates the data in the database. If the document doesn't exist, it will fail.
*
* NOTE: Update will skip any conversions and directly set the data.
* If you rely on the converter/conversion functionality, use set() with merge: true instead of update.
*
* @param data
*/
Expand Down Expand Up @@ -91,17 +94,18 @@ export function mapDataFromSnapshot<T>(options?: SnapshotOptions): OperatorFunct
* First checks that the data exists before writing to the datastore.
*
* If it does not exist, will call set without merge options in order to fully initialize the object's data.
* If it does exist, update is done on all defined values.
* If it does exist, update is done using set + merge on all defined values.
*
* @param data
*/
export type CreateOrUpdateWithAccessorFunction<T> = (data: Partial<T>) => Promise<WriteResult | void>;
export type CreateOrUpdateWithAccessorSetFunction<T> = (data: Partial<T>) => Promise<WriteResult | void>;

export function createOrUpdateWithAccessor<T>(accessor: FirestoreDocumentDataAccessor<T>): (data: Partial<T>) => Promise<WriteResult | void> {
export function createOrUpdateWithAccessorSet<T>(accessor: FirestoreDocumentDataAccessor<T>): (data: Partial<T>) => Promise<WriteResult | void> {
return (data: Partial<T>) => {
return accessor.exists().then((exists) => {
if (exists) {
return accessor.update(filterUndefinedValues(data) as UpdateData<T>);
const update = filterUndefinedValues(data);
return accessor.set(update, { merge: true });
} else {
return accessor.set(data as WithFieldValue<T>);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Observable } from 'rxjs';
import { FirestoreAccessorDriverRef } from '../driver/accessor';
import { DocumentReference, CollectionReference, Transaction, WriteBatch, DocumentSnapshot, SnapshotOptions, WriteResult } from '../types';
import { createOrUpdateWithAccessor, dataFromSnapshotStream, FirestoreDocumentDataAccessor } from './accessor';
import { createOrUpdateWithAccessorSet, dataFromSnapshotStream, FirestoreDocumentDataAccessor } from './accessor';
import { CollectionReferenceRef, DocumentReferenceRef } from '../reference';
import { FirestoreDocumentContext } from './context';

Expand Down Expand Up @@ -44,7 +44,7 @@ export abstract class AbstractFirestoreDocument<T,
}

createOrUpdate(data: Partial<T>): Promise<WriteResult | void> {
return createOrUpdateWithAccessor(this.accessor)(data);
return createOrUpdateWithAccessorSet(this.accessor)(data);
}

}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ISO8601DateString, makeModelFieldMapFunctions } from '@dereekb/util';
import { isValid } from 'date-fns';
import { firestoreDate, firestoreField } from './snapshot.field';

describe('firestoreField()', () => {
Expand Down Expand Up @@ -68,9 +69,10 @@ describe('firestoreDate()', () => {
const converted = dateField.from!.convert!(dateString);
expect(converted).toBeDefined();
expect(converted?.getTime()).toBe(value.getTime());
expect(isValid(converted)).toBe(true);
});

it('should convert data from a date string to a .', () => {
it('should convert data from a date to a date string.', () => {
const dateString = '2021-08-16T05:00:00.000Z';
const value = new Date(dateString);

Expand Down
4 changes: 4 additions & 0 deletions packages/firebase/src/test/common/test.driver.accessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,8 @@ export function describeAccessorTests<T>(init: () => DescribeAccessorTests<T>) {
}
});

// todo: test that update does not call the converter when setting values.

});

describe('set()', () => {
Expand Down Expand Up @@ -363,6 +365,8 @@ export function describeAccessorTests<T>(init: () => DescribeAccessorTests<T>) {
expect(c.hasDataFromUpdate(snapshot.data())).toBe(true);
});

// todo: test that set calls the converter when setting values.

});

describe('delete()', () => {
Expand Down

0 comments on commit 243d0d3

Please sign in to comment.