-
Notifications
You must be signed in to change notification settings - Fork 4
ENG-1475 Export relations to imported nodes #817
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: eng-1476-repair-lint-ci-task-for-database
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Missing legacy Same root cause as in Root Cause and ImpactThis function is used by (Refers to lines 717-720) Was this helpful? React with 👍 or 👎 to provide feedback. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -423,7 +423,7 @@ export const syncAllNodesAndRelations = async ( | |
| } | ||
| console.debug("Supabase client:", supabaseClient); | ||
|
|
||
| const allNodes = await collectDiscourseNodesFromVault(plugin); | ||
| const allNodes = await collectDiscourseNodesFromVault(plugin, true); | ||
|
|
||
| const changedNodeInstances = relationsOnly | ||
| ? [] | ||
|
Comment on lines
+426
to
429
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Imported nodes are unintentionally synced as Content and Concepts to the user's own space Changing
Root Cause and ImpactThe intent of passing Impact: Every sync (when The fix should separate the two uses: pass only non-imported nodes to (Refers to lines 426-434) Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
@@ -495,7 +495,7 @@ const convertDgToSupabaseConcepts = async ({ | |
| const nodeTypes = plugin.settings.nodeTypes ?? []; | ||
| const relationTypes = plugin.settings.relationTypes ?? []; | ||
| const discourseRelations = plugin.settings.discourseRelations ?? []; | ||
| allNodes = allNodes ?? (await collectDiscourseNodesFromVault(plugin)); | ||
| allNodes = allNodes ?? (await collectDiscourseNodesFromVault(plugin, true)); | ||
| const allNodesById = Object.fromEntries( | ||
| allNodes.map((n) => [n.nodeInstanceId, n]), | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| CREATE OR REPLACE FUNCTION public.rid_to_space_id_and_local_id(rid VARCHAR) | ||
| RETURNS public.accessible_resource STRICT STABLE | ||
| SET search_path = '' | ||
| LANGUAGE plpgsql AS $$ | ||
| DECLARE | ||
| uri VARCHAR; | ||
| source_local_id VARCHAR; | ||
| source_id BIGINT; | ||
| BEGIN | ||
| source_local_id := split_part(rid, '/', -1); | ||
| IF length(source_local_id) = length(rid) THEN | ||
| RETURN (null, 'Not a Rid')::public.accessible_resource; | ||
| END IF; | ||
| uri := substr(rid, 1, length(rid) - length(source_local_id) - 1); | ||
| IF rid ~ '^orn:\w+\.\w+:.*$' THEN | ||
| uri := concat(split_part(split_part(uri, ':', 2), '.', 1), ':', split_part(uri, ':', 3)); | ||
| ELSE | ||
| IF rid ~ '^orn:\w+:.*$' THEN | ||
| uri := substr(uri, 5); | ||
| END IF; | ||
| END IF; | ||
| SELECT id INTO source_id FROM public."Space" where url=uri; | ||
| IF source_id IS NULL THEN | ||
| RETURN (null, concat('Cannot find ', uri))::public.accessible_resource; | ||
| END IF; | ||
| RETURN (source_id, source_local_id); | ||
| END; | ||
| $$; | ||
|
|
||
| CREATE OR REPLACE FUNCTION public.rid_or_local_id_to_concept_db_id(rid VARCHAR, default_space_id BIGINT) | ||
| RETURNS BIGINT STRICT STABLE | ||
| SET search_path = '' | ||
| LANGUAGE plpgsql AS $$ | ||
| DECLARE r public.accessible_resource; | ||
| BEGIN | ||
| r := (SELECT public.rid_to_space_id_and_local_id(rid)); | ||
| IF r.space_id IS NULL THEN | ||
| RETURN (SELECT id FROM public."Concept" WHERE space_id = default_space_id AND source_local_id = rid); | ||
| ELSE | ||
| RETURN (SELECT id FROM public."Concept" WHERE space_id = r.space_id AND source_local_id = r.source_local_id); | ||
| END IF; | ||
| END; | ||
| $$; | ||
|
|
||
| CREATE OR REPLACE FUNCTION public._local_concept_to_db_concept(data public.concept_local_input) | ||
| RETURNS public."Concept" STABLE | ||
| SET search_path = '' | ||
| LANGUAGE plpgsql | ||
| AS $$ | ||
| DECLARE | ||
| concept public."Concept"%ROWTYPE; | ||
| reference_content JSONB := jsonb_build_object(); | ||
| key varchar; | ||
| value JSONB; | ||
| ref_single_val BIGINT; | ||
| ref_array_val BIGINT[]; | ||
| BEGIN | ||
| -- not fan of going through json, but not finding how to populate a record by a different shape record | ||
| concept := jsonb_populate_record(NULL::public."Concept", to_jsonb(data)); | ||
| IF data.author_local_id IS NOT NULL THEN | ||
| SELECT id FROM public."PlatformAccount" | ||
| WHERE account_local_id = data.author_local_id INTO concept.author_id; | ||
| END IF; | ||
| IF data.represented_by_id IS NOT NULL THEN | ||
| SELECT space_id, source_local_id FROM public."Content" | ||
| WHERE id = data.represented_by_id INTO concept.space_id, concept.source_local_id; | ||
| END IF; | ||
| IF data.space_url IS NOT NULL THEN | ||
| SELECT id FROM public."Space" | ||
| WHERE url = data.space_url INTO concept.space_id; | ||
| END IF; | ||
| IF concept.source_local_id = '' THEN | ||
| concept.source_local_id := NULL; | ||
| END IF; | ||
| IF data.represented_by_local_id = '' THEN | ||
| data.represented_by_local_id := NULL; | ||
| END IF; | ||
| IF data.schema_represented_by_local_id IS NOT NULL THEN | ||
| SELECT public.rid_or_local_id_to_concept_db_id( | ||
| data.schema_represented_by_local_id, concept.space_id) INTO concept.schema_id; | ||
| END IF; | ||
| concept.source_local_id = COALESCE(concept.source_local_id, data.represented_by_local_id); -- legacy input field | ||
| concept.reference_content := coalesce(data.reference_content, '{}'::jsonb); | ||
| IF data.local_reference_content IS NOT NULL THEN | ||
| FOR key, value IN SELECT * FROM jsonb_each(data.local_reference_content) LOOP | ||
| IF jsonb_typeof(value) = 'array' THEN | ||
| WITH el AS (SELECT jsonb_array_elements_text(value) as x), | ||
| el2 AS (SELECT public.rid_or_local_id_to_concept_db_id(x, concept.space_id) AS id FROM el) | ||
| SELECT array_agg(DISTINCT el2.id) INTO STRICT ref_array_val | ||
| FROM el2 WHERE el2.id IS NOT NULL; | ||
| reference_content := jsonb_set(reference_content, ARRAY[key], to_jsonb(ref_array_val)); | ||
| ELSIF jsonb_typeof(value) = 'string' THEN | ||
| SELECT public.rid_or_local_id_to_concept_db_id(value #>> '{}', concept.space_id) INTO STRICT ref_single_val; | ||
| reference_content := jsonb_set(reference_content, ARRAY[key], to_jsonb(ref_single_val)); | ||
| ELSE | ||
| RAISE EXCEPTION 'Invalid value in local_reference_content % %', value, jsonb_typeof(value); | ||
| END IF; | ||
| END LOOP; | ||
| concept.reference_content := concept.reference_content || reference_content; | ||
| END IF; | ||
| RETURN concept; | ||
| END; | ||
| $$; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡
!resourceIdsis always false because a Set object is always truthyThe guard
if (!resourceIds) return;at line 204 is intended to skip the Supabase upsert when there are no resource IDs to publish. However,resourceIdsis aSet<string>(initialized on line 165), and a Set object is always truthy in JavaScript regardless of its size. This means the check never triggers an early return.Root Cause and Impact
The condition
!resourceIdsevaluates the truthiness of the Set object itself, not its size. Since any object (including an empty Set) is truthy in JavaScript,!resourceIdsis alwaysfalse.The correct check should be
if (resourceIds.size === 0) return;.Impact: When no matching relation triples are found, the function proceeds to call
client.from("ResourceAccess").upsert(...)with an empty array ([...resourceIds.values()]yields[]). Depending on how Supabase handles an empty upsert, this could be a no-op or could cause an unnecessary network request. More importantly, the function then proceeds toloadRelations, iterate over relations, and potentially callsaveRelations— all unnecessary work.Was this helpful? React with 👍 or 👎 to provide feedback.