diff --git a/app/config/collections/platform.php b/app/config/collections/platform.php index db6bc29d10..d429f11ff1 100644 --- a/app/config/collections/platform.php +++ b/app/config/collections/platform.php @@ -2261,7 +2261,7 @@ $platformCollections = [ 'filters' => ['json'], ], [ - // Virtual attribute — CTAs live in their own `ctas` collection + // Virtual attribute — CTAs live in the `insightCTAs` collection // back-referenced by `insightInternalId`. The subQuery filter // joins them in at read time, so consumers still see them // embedded on the insight response. @@ -2418,18 +2418,6 @@ $platformCollections = [ 'array' => false, 'filters' => [], ], - [ - // Caller-supplied identifier, unique within the parent insight. - '$id' => ID::custom('key'), - 'type' => Database::VAR_STRING, - 'format' => '', - 'size' => Database::LENGTH_KEY, - 'signed' => true, - 'required' => true, - 'default' => null, - 'array' => false, - 'filters' => [], - ], [ '$id' => ID::custom('label'), 'type' => Database::VAR_STRING, @@ -2492,14 +2480,6 @@ $platformCollections = [ 'lengths' => [Database::LENGTH_KEY, 0], 'orders' => [], ], - [ - // Enforce per-insight key uniqueness at the DB layer. - '$id' => ID::custom('_key_insight_key'), - 'type' => Database::INDEX_UNIQUE, - 'attributes' => ['insightInternalId', 'key'], - 'lengths' => [0, Database::LENGTH_KEY], - 'orders' => [], - ], ], ], ]; diff --git a/src/Appwrite/Insights/Validator/CTAs.php b/src/Appwrite/Insights/Validator/CTAs.php index 565b4be93f..848253bc13 100644 --- a/src/Appwrite/Insights/Validator/CTAs.php +++ b/src/Appwrite/Insights/Validator/CTAs.php @@ -8,7 +8,7 @@ class CTAs extends Validator { public const MAX_COUNT_DEFAULT = 16; - protected string $message = 'Value must be an array of CTA descriptors. Each entry must define `key`, `label`, `service`, `method`, and an optional `params` object.'; + protected string $message = 'Value must be an array of CTA descriptors. Each entry must define `label`, `service`, `method`, and an optional `params` object.'; public function __construct(protected int $maxCount = self::MAX_COUNT_DEFAULT) { @@ -45,7 +45,7 @@ class CTAs extends Validator return false; } - foreach (['key', 'label', 'service', 'method'] as $required) { + foreach (['label', 'service', 'method'] as $required) { if (!isset($entry[$required]) || !\is_string($entry[$required]) || $entry[$required] === '') { return false; } diff --git a/src/Appwrite/Platform/Modules/Insights/Http/Manager/Insights/Create.php b/src/Appwrite/Platform/Modules/Insights/Http/Manager/Insights/Create.php index 1688867a6f..17568e15d0 100644 --- a/src/Appwrite/Platform/Modules/Insights/Http/Manager/Insights/Create.php +++ b/src/Appwrite/Platform/Modules/Insights/Http/Manager/Insights/Create.php @@ -85,7 +85,7 @@ class Create extends Action ->param('title', '', new Text(256), 'Short, human-readable title.') ->param('summary', '', new Text(4096, 0), 'Markdown summary describing the insight.', true) ->param('payload', null, new Nullable(new JSON()), 'Type-specific structured payload.', true) - ->param('ctas', [], new CTAsValidator(), 'Array of call-to-action descriptors. Each must contain `key` (unique within the insight), `label`, `service`, `method`, and an optional `params` object.', true) + ->param('ctas', [], new CTAsValidator(), 'Array of call-to-action descriptors. Each must contain `label`, `service`, `method`, and an optional `params` object.', true) ->param('analyzedAt', null, new Nullable(new DatetimeValidator()), 'Time the insight was analyzed in ISO 8601 format. Defaults to now.', true) ->inject('response') ->inject('project') @@ -129,18 +129,10 @@ class Create extends Action $reportInternalId = $report->getSequence(); } - $seen = []; $normalizedCTAs = []; foreach ($ctas as $cta) { - $key = (string) $cta['key']; - if (isset($seen[$key])) { - throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'CTA `key` values must be unique within an insight.'); - } - $seen[$key] = true; - $normalizedCTAs[] = [ - 'key' => $key, 'label' => (string) $cta['label'], 'service' => (string) $cta['service'], 'method' => (string) $cta['method'], @@ -182,7 +174,6 @@ class Create extends Action 'projectId' => $project->getId(), 'insightInternalId' => $insight->getSequence(), 'insightId' => $insight->getId(), - 'key' => $cta['key'], 'label' => $cta['label'], 'service' => $cta['service'], 'method' => $cta['method'], diff --git a/src/Appwrite/Utopia/Response/Model/InsightCTA.php b/src/Appwrite/Utopia/Response/Model/InsightCTA.php index 2fd493cd57..75f42a9bf8 100644 --- a/src/Appwrite/Utopia/Response/Model/InsightCTA.php +++ b/src/Appwrite/Utopia/Response/Model/InsightCTA.php @@ -34,12 +34,6 @@ class InsightCTA extends Model 'default' => '', 'example' => '5e5ea5c16897e', ]) - ->addRule('key', [ - 'type' => self::TYPE_STRING, - 'description' => 'Caller-supplied identifier, unique within the parent insight.', - 'default' => '', - 'example' => 'createIndex', - ]) ->addRule('label', [ 'type' => self::TYPE_STRING, 'description' => 'Human-readable label for the CTA, used in UI.', diff --git a/tests/e2e/Services/Insights/InsightsBase.php b/tests/e2e/Services/Insights/InsightsBase.php index 032dc2d7b4..d0ddaac89d 100644 --- a/tests/e2e/Services/Insights/InsightsBase.php +++ b/tests/e2e/Services/Insights/InsightsBase.php @@ -84,10 +84,9 @@ trait InsightsBase * - `documentsDB` → service `documentsDB`, method `createIndex` (params use collectionId/attributes) * - `vectorsDB` → service `vectorsDB`, method `createIndex` (params use collectionId/attributes) */ - protected function sampleCTA(string $key = 'createIndex', string $engine = 'tablesDB'): array + protected function sampleCTA(string $engine = 'tablesDB'): array { $base = [ - 'key' => $key, 'label' => 'Create missing index', 'method' => 'createIndex', ]; @@ -169,7 +168,7 @@ trait InsightsBase 'title' => 'Missing index on collection orders', 'summary' => 'Queries against `orders.status` are scanning the full collection.', 'payload' => ['databaseId' => 'main', 'engine' => $engine], - 'ctas' => [$this->sampleCTA('createIndex', $engine)], + 'ctas' => [$this->sampleCTA($engine)], ]; if ($reportId !== null) { @@ -349,7 +348,6 @@ trait InsightsBase $this->assertSame('orders', $insight['body']['parentResourceId']); $this->assertSame('Missing index on collection orders', $insight['body']['title']); $this->assertCount(1, $insight['body']['ctas']); - $this->assertSame('createIndex', $insight['body']['ctas'][0]['key']); $this->assertSame($insightId, $insight['body']['ctas'][0]['insightId']); $this->assertSame('Create missing index', $insight['body']['ctas'][0]['label']); $this->assertSame('tablesDB', $insight['body']['ctas'][0]['service']); @@ -475,7 +473,7 @@ trait InsightsBase $this->assertSame('report_not_found', $insight['body']['type']); } - public function testCreateRejectsDuplicateCTAIds(): void + public function testCreateRejectsCTAWithEmptyLabel(): void { $insight = $this->createInsight([ 'insightId' => ID::unique(), @@ -484,25 +482,7 @@ trait InsightsBase 'resourceId' => 'main', 'title' => 'Should not be created', 'ctas' => [ - ['key' => 'dup', 'label' => 'A', 'service' => 'databases', 'method' => 'createIndex'], - ['key' => 'dup', 'label' => 'B', 'service' => 'databases', 'method' => 'createIndex'], - ], - ]); - - $this->assertSame(400, $insight['headers']['status-code']); - $this->assertSame('general_argument_invalid', $insight['body']['type']); - } - - public function testCreateRejectsCTAWithEmptyFields(): void - { - $insight = $this->createInsight([ - 'insightId' => ID::unique(), - 'type' => 'databaseIndex', - 'resourceType' => 'databases', - 'resourceId' => 'main', - 'title' => 'Should not be created', - 'ctas' => [ - ['key' => '', 'label' => 'Has empty id', 'service' => 'databases', 'method' => 'createIndex'], + ['label' => '', 'service' => 'databases', 'method' => 'createIndex'], ], ]); @@ -518,7 +498,7 @@ trait InsightsBase 'resourceId' => 'main', 'title' => 'Should not be created', 'ctas' => [ - ['key' => 'createIndex', 'label' => 'Missing method', 'service' => 'tablesDB'], + ['label' => 'Missing method', 'service' => 'tablesDB'], ], ]); @@ -534,7 +514,7 @@ trait InsightsBase 'resourceId' => 'main', 'title' => 'Should not be created', 'ctas' => [ - ['key' => 'createIndex', 'label' => 'Missing service', 'method' => 'createIndex'], + ['label' => 'Missing service', 'method' => 'createIndex'], ], ]); @@ -546,7 +526,6 @@ trait InsightsBase $ctas = []; for ($i = 0; $i < 17; $i++) { $ctas[] = [ - 'key' => 'cta-' . $i, 'label' => 'CTA ' . $i, 'service' => 'databases', 'method' => 'createIndex', diff --git a/tests/unit/Insights/Validator/CTAsTest.php b/tests/unit/Insights/Validator/CTAsTest.php index fbc30cad81..d1208da510 100644 --- a/tests/unit/Insights/Validator/CTAsTest.php +++ b/tests/unit/Insights/Validator/CTAsTest.php @@ -28,7 +28,6 @@ class CTAsTest extends TestCase $validator = new CTAs(); $this->assertTrue($validator->isValid([[ - 'key' => 'createIndex', 'label' => 'Create missing index', 'service' => 'tablesDB', 'method' => 'createIndex', @@ -44,7 +43,6 @@ class CTAsTest extends TestCase $validator = new CTAs(); $this->assertTrue($validator->isValid([[ - 'key' => 'createIndex', 'label' => 'Create missing index', 'service' => 'tablesDB', 'method' => 'createIndex', @@ -55,10 +53,9 @@ class CTAsTest extends TestCase { $validator = new CTAs(); - $this->assertFalse($validator->isValid([['key' => 'x']])); - $this->assertFalse($validator->isValid([['key' => 'x', 'label' => 'y']])); - $this->assertFalse($validator->isValid([['key' => 'x', 'label' => 'y', 'service' => 'tablesDB']])); - $this->assertFalse($validator->isValid([['key' => 'x', 'label' => 'y', 'method' => 'createIndex']])); + $this->assertFalse($validator->isValid([['label' => 'x']])); + $this->assertFalse($validator->isValid([['label' => 'x', 'service' => 'tablesDB']])); + $this->assertFalse($validator->isValid([['label' => 'x', 'method' => 'createIndex']])); } public function testRejectsEntryWithEmptyStrings(): void @@ -66,8 +63,7 @@ class CTAsTest extends TestCase $validator = new CTAs(); $this->assertFalse($validator->isValid([[ - 'key' => '', - 'label' => 'Create missing index', + 'label' => '', 'service' => 'tablesDB', 'method' => 'createIndex', ]])); @@ -78,8 +74,7 @@ class CTAsTest extends TestCase $validator = new CTAs(); $this->assertFalse($validator->isValid([[ - 'key' => 123, - 'label' => 'Create missing index', + 'label' => 123, 'service' => 'tablesDB', 'method' => 'createIndex', ]])); @@ -90,7 +85,6 @@ class CTAsTest extends TestCase $validator = new CTAs(); $this->assertFalse($validator->isValid([[ - 'key' => 'createIndex', 'label' => 'Create missing index', 'service' => 'tablesDB', 'method' => 'createIndex', @@ -113,7 +107,6 @@ class CTAsTest extends TestCase $entries = []; for ($i = 0; $i < 4; $i++) { $entries[] = [ - 'key' => 'cta-' . $i, 'label' => 'Label ' . $i, 'service' => 'tablesDB', 'method' => 'createIndex', @@ -131,7 +124,6 @@ class CTAsTest extends TestCase $entries = []; for ($i = 0; $i < 3; $i++) { $entries[] = [ - 'key' => 'cta-' . $i, 'label' => 'Label ' . $i, 'service' => 'tablesDB', 'method' => 'createIndex', @@ -146,7 +138,6 @@ class CTAsTest extends TestCase $validator = new CTAs(); $entry = [ - 'key' => 'createIndex', 'label' => 'Create missing index', 'service' => 'tablesDB', 'method' => 'createIndex', @@ -161,7 +152,6 @@ class CTAsTest extends TestCase $validator = new CTAs(); $this->assertFalse($validator->isValid([[ - 'key' => 'createIndex', 'label' => 'Create missing index', 'service' => '', 'method' => 'createIndex', @@ -173,25 +163,12 @@ class CTAsTest extends TestCase $validator = new CTAs(); $this->assertFalse($validator->isValid([[ - 'key' => 'createIndex', 'label' => 'Create missing index', 'service' => 'tablesDB', 'method' => '', ]])); } - public function testRejectsEntryWithEmptyLabel(): void - { - $validator = new CTAs(); - - $this->assertFalse($validator->isValid([[ - 'key' => 'createIndex', - 'label' => '', - 'service' => 'tablesDB', - 'method' => 'createIndex', - ]])); - } - public function testDefaultMaxCountIsSixteen(): void { $validator = new CTAs(); @@ -201,7 +178,6 @@ class CTAsTest extends TestCase $entries = []; for ($i = 0; $i < 16; $i++) { $entries[] = [ - 'key' => 'cta-' . $i, 'label' => 'Label ' . $i, 'service' => 'tablesDB', 'method' => 'createIndex', @@ -211,7 +187,6 @@ class CTAsTest extends TestCase $this->assertTrue($validator->isValid($entries)); $entries[] = [ - 'key' => 'cta-16', 'label' => 'Label 16', 'service' => 'tablesDB', 'method' => 'createIndex',