Skip to content

Conversation

@geertplaisier
Copy link
Collaborator

@geertplaisier geertplaisier commented Oct 31, 2025

No description provided.

Copy link
Contributor

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 introduces a flexible attribute list source architecture that allows the attribute list component to consume data from multiple sources beyond the default Tailormap API. The refactoring abstracts data loading through an AttributeListSourceModel interface, enabling extensibility for custom data sources.

Key Changes:

  • Added a source-based architecture to the attribute list component, allowing multiple data sources
  • Introduced AttributeListSourceModel interface and AttributeListApiServiceModel to abstract data loading
  • Updated AttributeListTabModel to include tabSourceId to track which source a tab belongs to
  • Modified service layer to route API calls through AttributeListManagerService based on tab source IDs

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
attribute-list-manager.service.ts Refactored to manage multiple sources, added methods to register sources and route API calls based on tabSourceId
attribute-list-api.service.ts New service implementing AttributeListApiServiceModel as a wrapper for the default Tailormap API
attribute-list-source.model.ts New model defining the interface for attribute list data sources
attribute-list-api-service.model.ts New model defining parameter types and interface for data loading operations
attribute-list-default-source.const.ts Constant for the default source ID
attribute-list-tab.model.ts Added tabSourceId property to track source ownership
attribute-list.effects.ts Updated to use manager service for getFeatures$ calls with tabSourceId
attribute-list-data.service.ts Injected AttributeListManagerService instead of TAILORMAP_API_V1_SERVICE
attribute-list-export.service.ts Updated export methods to accept and pass tabSourceId
attribute-list-filter.component.ts Updated to use manager service for unique values lookup
attribute-list-export-button.component.ts Updated to extract tabSourceId from selected tab
attribute-list-content.component.ts Updated filter dialog to include tabSourceId
attribute-list.module.ts Added initialization of default attribute list source
Various test files Updated test data and setup to include tabSourceId
Comments suppressed due to low confidence (2)

projects/core/src/lib/components/attribute-list/services/attribute-list-data.service.spec.ts:118

  • The test expects api.getFeatures$ to be called with these parameters, but after the refactoring, the method signature changed to accept tabSourceId as the first parameter followed by the params object. The test assertion should be updated to match the new signature: api.getFeatures$(ATTRIBUTE_LIST_DEFAULT_SOURCE, { ... }).
      expect(api.getFeatures$).toHaveBeenCalledWith({
        layerId: '1',
        applicationId: '1',
        page: 0,
        filter: undefined,
        sortBy: undefined,
        sortOrder: undefined,
      });

projects/core/src/lib/components/attribute-list/state/mocks/attribute-list-state-test-data.ts:52

  • Missing space around the '+' operator. Should be i + 1 for consistency with project style guidelines.
    rows.push(createDummyRow(`${i+1}`, rowOverride ? rowOverride(i) : undefined));

@github-actions
Copy link

github-actions bot commented Oct 31, 2025

Test Results

  1 files  ±0  215 suites  ±0   8m 55s ⏱️ +27s
533 tests +1  533 ✅ +1  0 💤 ±0  0 ❌ ±0 
619 runs  +1  619 ✅ +1  0 💤 ±0  0 ❌ ±0 

Results for commit 61494bb. ± Comparison against base commit 5ed0f0b.

♻️ This comment has been updated with latest results.

@geertplaisier
Copy link
Collaborator Author

geertplaisier commented Nov 6, 2025

  • Improve PR by moving the datasource higher up in the application so it can also be used by for example feature info

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