ILEX-136 replace elasticsearch with entity-check #131

Aiga115 · 2025-09-09T12:16:10Z

Issue #ILEX-136
Problem: replace elasticsearch with entity-check
Solution:

Call entity-check endpoint to find potential match
If no potential match we call elastic search function and place terms on sidebar
Make term items on sidebar clickable and when clicking on one of the terms start edit term flow
Change button title and function call according to isEditing state

Note: We don't have api call for editing so now editTerm function just logs the message. Has to be connected later

Result:

Recording.2025-09-09.141349.mp4

Recording.2025-09-09.141502.mp4

Copilot

Pull Request Overview

This PR replaces the elasticsearch-based term matching system with a new entity-check API to identify potential term matches. When no exact matches are found through entity-check, the system falls back to elasticsearch for searching terms that can be selected for editing.

Replace elasticsearch search with entity-check API for finding potential matches
Enable term editing flow by making sidebar term items clickable
Add fallback to elasticsearch when no exact matches found via entity-check

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/constants/types.js	Adds term type constants and exports for OWL classes and properties
src/config.js	Adds CHECK_ENTITY endpoint configuration for the new API
src/components/TermEditor/newTerm/SecondStepContent.jsx	Updates component to accept searchTerm prop and use it in predicate initialization
src/components/TermEditor/newTerm/FirstStepContent.jsx	Major refactor to use entity-check API with elasticsearch fallback and enable editing mode
src/components/TermEditor/newTerm/AddNewTermDialog.jsx	Adds editing state management and switches between create/edit modes
src/components/TermEditor/NewTermSidebar.jsx	Improves loading states with skeletons and makes non-exact match items clickable
src/api/endpoints/apiService.ts	Adds checkPotentialMatches function and refactors createNewEntity to handle redirects properly

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

src/components/TermEditor/newTerm/FirstStepContent.jsx

Copilot · 2025-09-15T17:03:44Z

src/components/TermEditor/newTerm/FirstStepContent.jsx

+                            const matchList = Array.isArray(matches) ? matches : [matches];
+                            return matchList.map(match => ({
+                                ilx: termUri.split('/').pop(),
+                                label: match.object,


The code assumes match has an object property when creating the label field, but this property might not exist. This could result in undefined labels for exact matches.

Suggested change

label: match.object,

label: match.object ?? termUri.split('/').pop() ?? "Unknown",

@Aiga115 take this into account as well, thanks!

Copilot · 2025-09-15T17:03:44Z

src/components/TermEditor/NewTermSidebar.jsx

                                '&:hover': { backgroundColor: "transparent" }
                            }}
-                            onClick={() => onResultAction(result)}
+                            onClick={() => window.location.href = `/${user.groupname}/${result.ilx}`}


Using window.location.href for navigation breaks the React Router pattern and causes a full page reload. Consider using useNavigate hook from React Router instead.

@Aiga115 I agree with copilot suggestion here, thanks!

Copilot · 2025-09-15T17:03:45Z

src/api/endpoints/apiService.ts

+
+  return new Promise((resolve, reject) => {
+    const xhr = new XMLHttpRequest();
+
+    xhr.open('POST', endpoint, true);
+    xhr.setRequestHeader('Content-Type', 'application/json');
+    xhr.withCredentials = true;
+
+    xhr.onreadystatechange = function() {
+      if (xhr.readyState === XMLHttpRequest.HEADERS_RECEIVED) {
+        if (xhr.status === 303) {
+          const redirectUrl = xhr.getResponseHeader('x-redirect-location') || xhr.getResponseHeader('Location');          
+          if (redirectUrl) {
+            const tmpMatch = redirectUrl.match(/tmp_\d{9}/);
+            console.log("Found tmp ID:", tmpMatch?.[0]);
+
+            if (tmpMatch) {
+              resolve({
+                term: {
+                  id: tmpMatch[0]
+                }
+              });
+              return;
+            }
+          }
+        }
      }
-    }

-    // Otherwise, return response as-is
-    return response;
-  } catch (error) {
-    if (error?.response.status === 409) {
-      const match = error?.response?.data?.existing?.[0];
-      if (match) {
-        return {
-          term: {
-            id: `${match}`,
-          },
-          raw: error?.response,
-          status: error?.response?.status,
-        };
+      if (xhr.readyState === XMLHttpRequest.DONE) {
+        if (xhr.status === 0) {
+          // Try to find tmp ID in the response text
+          const tmpMatch = xhr.responseText?.match(/tmp_\d{9}/);
+          if (tmpMatch) {
+            resolve({
+              term: {
+                id: tmpMatch[0]
+              },
+              raw: xhr.responseText,
+              status: 303
+            });
+            return;
+          }
+        }
+
+        // Handle 409 Conflict
+        if (xhr.status === 409) {
+          try {
+            const responseData = JSON.parse(xhr.responseText);
+            const match = responseData?.existing?.[0];
+            if (match) {
+              resolve({
+                term: {
+                  id: match
+                },
+                raw: responseData,
+                status: xhr.status
+              });
+              return;
+            }
+          } catch (e) {
+            console.error("Error parsing 409 response:", e);
+          }
+        }
+
+        resolve({
+          raw: xhr.responseText,
+          status: xhr.status
+        });
      }
-    }
+    };

-    return {
-      raw: error?.response,
-      status: error?.response?.status,
+    xhr.onerror = function() {
+      console.error("XHR Error:", xhr.status, xhr.statusText);
+      reject(new Error('Network request failed'));
    };
-  }

-};
+    xhr.send(JSON.stringify(data));
+  });


Using XMLHttpRequest directly instead of the existing createPostRequest function breaks consistency with the rest of the codebase. Consider modifying createPostRequest to handle redirects instead of implementing a custom XHR solution.

src/components/TermEditor/NewTermSidebar.jsx

Copilot · 2025-09-15T17:03:45Z

src/components/TermEditor/newTerm/AddNewTermDialog.jsx

-    const navigate = useNavigate();

-    const isCreateButtonDisabled = hasExactMatch || termValue === "";
+    const isCreateButtonDisabled = hasExactMatch || termValue === "" || (isEditing && termValue === selectedTermValue);


[nitpick] The button disabled logic is complex and could be extracted into a separate function or useMemo for better readability and testability.

ddelpiano · 2025-09-15T17:04:24Z

src/api/endpoints/apiService.ts

+  const endpoint = `/${group}${API_CONFIG.REAL_API.CREATE_NEW_ENTITY}`;
+
+  return new Promise((resolve, reject) => {
+    const xhr = new XMLHttpRequest();


@Aiga115 why do not parametrise the content-type of createPostRequest but re-implement the entire request inside this function?

ddelpiano · 2025-09-15T17:06:18Z

src/api/endpoints/apiService.ts

+
+    xhr.onreadystatechange = function() {
+      if (xhr.readyState === XMLHttpRequest.HEADERS_RECEIVED) {
+        if (xhr.status === 303) {


Following the comment above, we should be able to handle these cases as part of the response of the createPostRequest rather than re-implementing ex novo the request on a case basis, so that all the code in common across requests will be handled by a single function and we avoid to duplicate code.

ddelpiano · 2025-09-15T17:06:56Z

src/api/endpoints/apiService.ts

 };
+
+export const checkPotentialMatches = async (group: string, data: any) => {
+  return createPostRequest<any, any>(`/${group}${API_CONFIG.REAL_API.CHECK_ENTITY}`, { "Content-Type": "application/json" })(data);


ddelpiano · 2025-09-15T20:32:05Z

src/components/TermEditor/NewTermSidebar.jsx

                                '&:hover': { backgroundColor: "transparent" }
                            }}
-                            onClick={() => onResultAction(result)}
+                            onClick={() => window.location.href = `/${user.groupname}/${result.ilx}`}


@Aiga115 I agree with copilot suggestion here, thanks!

ddelpiano · 2025-09-15T20:33:26Z

src/components/TermEditor/newTerm/FirstStepContent.jsx

+                            const matchList = Array.isArray(matches) ? matches : [matches];
+                            return matchList.map(match => ({
+                                ilx: termUri.split('/').pop(),
+                                label: match.object,


@Aiga115 take this into account as well, thanks!

ddelpiano · 2025-09-15T21:07:47Z

Interlex.webm

@Aiga115 when selecting a potential match from the sidebar the sidebar itself disappear (check the recording), this is not expected, the user might also have selected the wrong item, the sidebar needs to stay there and visible.

ddelpiano · 2025-09-15T21:11:30Z

@Aiga115 upon triggering the debounce, the elasticsearch endpoint is hit a second time that is unnecessary since the query term requested is the same, please fix this as well. Thanks

Screencast.from.15-09-25.23.09.57.webm

…ating a hook

ddelpiano · 2025-09-18T17:44:42Z

src/api/endpoints/apiActions.ts

+      });
+
+      // Handle 303 redirect
+      if (response.status === 303) {


This logic should be injected from whatever component calls the createPostRequest and wants to handle this differently, modifying this into this call implies that all the components in the application that are using the createPostRequest will have to behave the same way the add new term component needs to. Please fix this.

ddelpiano · 2025-09-18T17:44:50Z

src/api/endpoints/apiActions.ts

+
+      // Handle 409 Conflict
+      if (response.status === 409) {
+        const responseData = await response.json();


Aiga115 added 10 commits August 28, 2025 20:58

ILEX-136 add check for potential matches logic

a347bf7

Merge branch 'feature/ILEX-99' into feature/ILEX-136

d2eac8d

ILEX-136 replace elastic search with api endpoint

ae09eff

Merge branch 'feature/ILEX-99' into feature/ILEX-136

00093b3

ILEX-136 provide correct header in api call

ecfe7d0

Merge branch 'devel' into feature/ILEX-136

fb37131

ILEX-136 make change

3172caa

ILEX-136 change the way how to handle potential matches

35e9efd

ILEX-136 add skeleton loader to sidebar and fix connecting logic

5dc29db

ILEX-136 add edit term flow

b452112

Aiga115 requested a review from ddelpiano September 9, 2025 12:16

Merge branch 'devel' into feature/ILEX-136

9d3b5be

ddelpiano requested a review from Copilot September 15, 2025 17:02

Copilot AI reviewed Sep 15, 2025

View reviewed changes

ddelpiano requested changes Sep 15, 2025

View reviewed changes

ILEX-136 change post request logic and make code more readable by cre…

4ee954d

…ating a hook

Aiga115 marked this pull request as draft September 16, 2025 14:07

Aiga115 added 2 commits September 17, 2025 12:16

ILEX-136 put limit to elastic search

200f5ab

ILEX-136 fix linting

69ce8bc

Aiga115 marked this pull request as ready for review September 17, 2025 10:27

Aiga115 requested a review from ddelpiano September 17, 2025 10:27

Merge branch 'devel' into feature/ILEX-136

0d42912

ddelpiano requested changes Sep 18, 2025

View reviewed changes

ILEX-136 delete unnecessary logic

32b4f66

ddelpiano merged commit 54e35e4 into devel Sep 22, 2025
1 check passed

	label: match.object,
	label: match.object ?? termUri.split('/').pop() ?? "Unknown",

ILEX-136 replace elasticsearch with entity-check #131

ILEX-136 replace elasticsearch with entity-check #131

Uh oh!

Conversation

Aiga115 commented Sep 9, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ddelpiano commented Sep 15, 2025

Uh oh!

ddelpiano commented Sep 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants