Merge pull request #1 from david-mcafee/custom-pk-fix-delete

fix delete flow

packages/datastore/src/storage/storage.ts

@@ -25,6 +25,7 @@ import {
 	validatePredicate,
 	valuesEqual,
 } from '../util';
+import { getIdentifierValue } from '../sync/utils';
 import { Adapter } from './adapter';
 import getDefaultAdapter from './adapter/getDefaultAdapter';
 
@@ -175,7 +176,21 @@ class StorageClass implements StorageFacade {
 			condition
 		);
 
-		const modelIds = new Set(models.map(({ id }) => id));
+		const modelConstructor = isModelConstructor(modelOrModelConstructor)
+			? modelOrModelConstructor
+			: (Object.getPrototypeOf(modelOrModelConstructor || {})
+					.constructor as PersistentModelConstructor<T>);
+		const namespaceName = this.namespaceResolver(modelConstructor);
+
+		const modelDefinition =
+			this.schema.namespaces[namespaceName].models[modelConstructor.name];
+
+		const modelIds = new Set(
+			models.map(model => {
+				const modelId = getIdentifierValue(modelDefinition, model);
+				return modelId;
+			})
+		);
 
 		if (
 			!isModelConstructor(modelOrModelConstructor) &&
@@ -191,7 +206,8 @@ class StorageClass implements StorageFacade {
 			let theCondition: PredicatesGroup<any>;
 
 			if (!isModelConstructor(modelOrModelConstructor)) {
-				theCondition = modelIds.has(model.id)
+				const modelId = getIdentifierValue(modelDefinition, model);
+				theCondition = modelIds.has(modelId)
 					? ModelPredicateCreator.getPredicates(condition, false)
 					: undefined;
 			}

packages/datastore/src/sync/index.ts

@@ -356,7 +356,6 @@ export class SyncEngine {
 					.observe(null, null, ownSymbol)
 					.filter(({ model }) => {
 						const modelDefinition = this.getModelDefinition(model);
-
 						return modelDefinition.syncable === true;
 					})
 					.subscribe({
@@ -366,7 +365,11 @@ export class SyncEngine {
 							const MutationEventConstructor = this.modelClasses[
 								'MutationEvent'
 							] as PersistentModelConstructor<MutationEvent>;
-							const graphQLCondition = predicateToGraphQLCondition(condition);
+							const modelDefinition = this.getModelDefinition(model);
+							const graphQLCondition = predicateToGraphQLCondition(
+								condition,
+								modelDefinition
+							);
 							const mutationEvent = createMutationInstanceFromModelOperation(
 								namespace.relationships,
 								this.getModelDefinition(model),

packages/datastore/src/sync/utils.ts

@@ -27,7 +27,6 @@ import {
 	InternalSchema,
 	AuthModeStrategy,
 	extractPrimaryKeyFieldNames,
-	// isPrimaryKeyId,
 } from '../types';
 import { exhaustiveCheck } from '../util';
 import { MutationEvent } from './';
@@ -430,25 +429,36 @@ export function createMutationInstanceFromModelOperation<
 }
 
 export function predicateToGraphQLCondition(
-	predicate: PredicatesGroup<any>
+	predicate: PredicatesGroup<any>,
+	modelDefinition: SchemaModel
 ): GraphQLCondition {
 	const result = {};
 
 	if (!predicate || !Array.isArray(predicate.predicates)) {
 		return result;
 	}
 
+	const keyFields = extractPrimaryKeyFieldNames(modelDefinition);
+
 	predicate.predicates.forEach(p => {
 		if (isPredicateObj(p)) {
 			const { field, operator, operand } = p;
 
-			if (field === 'id') {
+			// This is compatible with how the GQL Transform currently generates the Condition Input,
+			// i.e. any PK and SK fields are omitted and can't be used as conditions.
+			// However, I think this limits usability.
+			// What if we want to delete all records where SK > some value
+			// Or all records where PK = some value but SKs are different values
+
+			// TODO: if the Transform gets updated ^ we'll need to modify this logic to only omit
+			// key fields from the predicate when ALL of the keyFields are present and using `eq` operators
+			if (typeof field === 'string' && keyFields.includes(field)) {
 				return;
 			}
 
 			result[field] = { [operator]: operand };
 		} else {
-			result[p.type] = predicateToGraphQLCondition(p);
+			result[p.type] = predicateToGraphQLCondition(p, modelDefinition);
 		}
 	});