fix: gen tools get entry action priority (#1621)

teamkeel · Oct 2, 2024 · 09d55a4 · 09d55a4
1 parent abf327e
commit 09d55a4
Show file tree

Hide file tree

Showing 4 changed files with 284 additions and 35 deletions.
diff --git a/tools/generator.go b/tools/generator.go
@@ -142,14 +142,20 @@ func (g *Generator) decorateTools() error {
 	// decorate further...
 	for _, tool := range g.Tools {
 		// for all inputs that are IDs that have a get_entry_action link (e.g. used to lookup a related model field),
-		// decorate the data mapping now that we have all inputs and responses generated
+		// find the get(id) tool and decorate the data mapping now that we have all inputs and responses generated
 		for _, input := range tool.Config.Inputs {
 			if input.GetEntryAction != nil && input.GetEntryAction.ToolId != "" {
-				input.GetEntryAction.Data = []*toolsproto.DataMapping{
-					{
-						Key:  g.Tools[input.GetEntryAction.ToolId].getIDInputFieldPath(),
-						Path: input.FieldLocation,
-					},
+				if entryToolID := g.findGetByIDTool(input.GetEntryAction.ToolId); entryToolID != "" {
+					input.GetEntryAction.ToolId = entryToolID
+					input.GetEntryAction.Data = []*toolsproto.DataMapping{
+						{
+							Key:  g.Tools[entryToolID].getIDInputFieldPath(),
+							Path: input.FieldLocation,
+						},
+					}
+				} else {
+					// if not get(id) is found, then remove the GetEntryAction placeholder
+					input.GetEntryAction = nil
 				}
 			}
 		}
@@ -445,12 +451,10 @@ func (g *Generator) makeInputsForMessage(actionType proto.ActionType, msg *proto
 			// create the GetEntry tool link to retrieve the entry for this related model. At this point, not all tools'
 			// inputs and responses have been generated ; this is a placeholder that will have it's data populated later
 			// in the generation process
-			if entryToolID := g.findGetTool(f.Type.ModelName.Value); entryToolID != "" {
-				// We do not add a GetEntryAction for the 'id' (or any unique lookup) input on a 'get', 'create' or 'update' action of a model, however do we add it for related models
-				if !((actionType == proto.ActionType_ACTION_TYPE_GET || actionType == proto.ActionType_ACTION_TYPE_CREATE || actionType == proto.ActionType_ACTION_TYPE_UPDATE) && len(f.Target) == 1) {
-					config.GetEntryAction = &toolsproto.ActionLink{
-						ToolId: entryToolID,
-					}
+			// We do not add a GetEntryAction for the 'id' (or any unique lookup) input on a 'get', 'create' or 'update' action of a model, however do we add it for related models
+			if !((actionType == proto.ActionType_ACTION_TYPE_GET || actionType == proto.ActionType_ACTION_TYPE_CREATE || actionType == proto.ActionType_ACTION_TYPE_UPDATE) && len(f.Target) == 1) {
+				config.GetEntryAction = &toolsproto.ActionLink{
+					ToolId: f.Type.ModelName.Value, // TODO: this is a bit of a hack placeholder because we do not know the underlying model which the field is pointing to during post-processing
 				}
 			}
 		}
@@ -677,38 +681,36 @@ func (g *Generator) findListTools(modelName string) []string {
 	return ids
 }
 
-// findGetTool will search for a get tool for the given model. It will prioritise a get(id) action.
-func (g *Generator) findGetTool(modelName string) string {
-	for id, tool := range g.Tools {
-		if tool.Model.Name == modelName && tool.Action.IsGet() && tool.hasOnlyIDInput() {
-			return id
-		}
-	}
-
+// findCreateTool will search for a get tool for the given model
+func (g *Generator) findCreateTool(modelName string) string {
 	for id, tool := range g.Tools {
-		if tool.Model.Name == modelName && tool.Action.IsGet() {
+		if tool.Model.Name == modelName && tool.Action.IsCreate() {
 			return id
 		}
 	}
 
 	return ""
 }
 
-// findCreateTool will search for a get tool for the given model
-func (g *Generator) findCreateTool(modelName string) string {
+// findGetByIDTool will search for a get tool for the given model that takes in an ID. It will prioritise a get(id) action without @embeds
+func (g *Generator) findGetByIDTool(modelName string) string {
+	if id := g.findGetByIDWithoutEmbedsTool(modelName); id != "" {
+		return id
+	}
+
 	for id, tool := range g.Tools {
-		if tool.Model.Name == modelName && tool.Action.IsCreate() {
+		if tool.Model.Name == modelName && tool.Action.IsGet() && tool.hasOnlyIDInput() {
 			return id
 		}
 	}
 
 	return ""
 }
 
-// findGetByIDTool will search for a get tool for the given model that takes in an ID
-func (g *Generator) findGetByIDTool(modelName string) string {
+// findGetByIDTool will search for a get tool for the given model that takes in an ID and has no @embeds defined
+func (g *Generator) findGetByIDWithoutEmbedsTool(modelName string) string {
 	for id, tool := range g.Tools {
-		if tool.Model.Name == modelName && tool.Action.IsGet() && tool.hasOnlyIDInput() {
+		if tool.Model.Name == modelName && tool.Action.IsGet() && tool.hasOnlyIDInput() && len(tool.Action.ResponseEmbeds) == 0 {
 			return id
 		}
 	}

diff --git a/tools/testdata/input_get_entry_actions/schema.keel b/tools/testdata/input_get_entry_actions/schema.keel
@@ -2,10 +2,15 @@ model Product {
     fields {
         name Text
         sku Text @unique
+        supplier Supplier
     }
 
     actions {
         get getProduct(id)
+        get getProductBySku(sku)
+        get getProductWithSupplier(id) {
+            @embed(supplier)
+        }
 
         list listProducts()
         update updateProduct(id)
@@ -14,3 +19,4 @@ model Product {
     }
 }
 
+model Supplier {}