Skip to content

Dynamic URL that mirros interlex urls + optimizations#118

Merged
ddelpiano merged 4 commits intodevelfrom
feature/ILEX-103
Jul 14, 2025
Merged

Dynamic URL that mirros interlex urls + optimizations#118
ddelpiano merged 4 commits intodevelfrom
feature/ILEX-103

Conversation

@ddelpiano
Copy link
Member

No description provided.

@ddelpiano ddelpiano requested a review from Copilot July 14, 2025 15:53
@ddelpiano ddelpiano changed the base branch from main to devel July 14, 2025 15:53
@ddelpiano ddelpiano requested a review from jrmartin July 14, 2025 15:53

This comment was marked as outdated.


const handleGoToTermClick = () => {
navigate(`/view?searchTerm=${newTermId}`);
navigate(`/base/${newTermId}/overview`);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ddelpiano Should it be the username here instead of base?

@ddelpiano ddelpiano requested review from Copilot and jrmartin July 14, 2025 17:45
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances URL routing to include dynamic user group segments, implements fallback logic for API requests, and applies memoization and callbacks across term views to optimize performance.

  • Updated routing and navigation to use :group URL parameter and dynamic groupName from user context.
  • Refactored SingleTermView to use URL params, optimized data fetching with caching and fallback, and replaced query-based logic.
  • Extended API service methods (getSelectedTermLabel, getMatchTerms, getRawData) with fallback to the ‘base’ group and enriched error handling.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
AddNewTermDialogContent.jsx Added user context and updated navigation to include groupName.
SingleTermView/index.jsx Refactored component to use URL params, dynamic grouping, caching, and memoized handlers.
OverView/OverView.jsx Added group prop support in data fetch calls and propTypes.
SearchResults/ListView.jsx Replaced static /view links with dynamic group-based URLs.
Header/index.jsx Updated user nav menu to generate group-aware dashboard link.
Header/Search.jsx Updated search navigation to include dynamic group segments.
GraphViewer/GraphStructure.jsx Added clarifying comments in object-predicate logic.
api/endpoints/apiService.ts Introduced fallback logic for API calls across all group-based endpoints.
App.jsx Modified routes to include :group param for search, dashboard, and term views.
Comments suppressed due to low confidence (5)

src/components/Header/index.jsx:167

  • The "Log out" menu item lacks an href or click handler to perform the logout action. Consider adding a proper navigation path or onClick callback so users can actually log out.
            label: 'Log out',

src/components/SingleTermView/index.jsx:228

  • [nitpick] The variable name isItFork is unclear. Consider renaming it to something like isForked or isExternalFork for better readability.
  const isItFork = actualGroup === 'base' ? false : true; // Use actualGroup instead of group

src/api/endpoints/apiService.ts:87

  • You’ve introduced fallback logic for non-‘base’ groups here. It would be helpful to add unit tests covering both successful and fallback request scenarios to ensure correct behavior.
    if (group !== 'base') {

src/components/Header/index.jsx:157

  • user is referenced here but not defined in this scope. You need to add const { user } = useContext(GlobalDataContext); and import GlobalDataContext at the top of this component.
        return user?.groupname || 'base';

src/components/SingleTermView/OverView/OverView.jsx:25

  • [nitpick] This console.log appears to be a leftover debug statement. Remove it or replace with proper logging if needed to reduce noise in production.
          console.log("data from api call: ", data)

@ddelpiano ddelpiano merged commit bf7f2d8 into devel Jul 14, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants