Skip to content

Feature/matb/add searchexternalid appconfig#670

Merged
matbmapspeople merged 4 commits intomainfrom
feature/matb/add_searchexternalid_appconfig
Mar 31, 2026
Merged

Feature/matb/add searchexternalid appconfig#670
matbmapspeople merged 4 commits intomainfrom
feature/matb/add_searchexternalid_appconfig

Conversation

@matbmapspeople
Copy link
Copy Markdown
Collaborator

@matbmapspeople matbmapspeople commented Mar 31, 2026

Summary by CodeRabbit

  • Bug Fixes
    • External locations search configuration has been improved to provide additional flexibility. The feature can now be configured through App Config, in addition to all existing configuration methods. This enhancement allows teams to better manage their configuration across different deployment environments, improving overall setup flexibility and simplifying the configuration management process.

…nalLocations handling with appConfig integration
…y and crossorigin attributes for improved security
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

📝 Walkthrough

Walkthrough

Added support for configuring searchExternalLocations via App Config. The MapTemplate component now prioritizes app configuration over component props when determining this setting, with fallback to the prop value if app config is unavailable.

Changes

Cohort / File(s) Summary
Documentation
packages/map-template/CHANGELOG.md
Added changelog entry documenting version 1.96.21 fix allowing searchExternalLocations configuration through App Config.
Component Configuration Logic
packages/map-template/src/components/MapTemplate/MapTemplate.jsx
Updated Recoil state effect to check appConfig.appSettings.searchExternalLocations first; converts string 'true' to boolean. Falls back to prop value if app config undefined. Added appConfig to effect dependencies.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

  • Lucaci-Andrei

Poem

🐰 A config path newly found,
App settings now abound,
External locations free to roam,
Through settings they now find their home! 🗺️✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title mentions 'searchexternalid' but the actual change is about 'searchExternalLocations' - a different property with a similar name pattern. The title is misleading about the specific property being modified. Update the title to accurately reflect the change: use 'searchExternalLocations' instead of 'searchexternalid' to clearly identify the correct property being configured via App Config.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/matb/add_searchexternalid_appconfig

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

…` configuration via App Config in version 1.96.21
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/map-template/CHANGELOG.md (1)

8-12: Consider using "Added" category and improving grammar.

This change adds new configuration capability rather than fixing a bug. Also, "can be now also" reads awkwardly.

✏️ Suggested changelog entry
 ## [1.96.21] - 2026-03-31

-### Fixed
+### Added

-- `searchExternalLocations` can be now also set via App Config.
+- `searchExternalLocations` can now also be set via App Config.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/map-template/CHANGELOG.md` around lines 8 - 12, Update the changelog
entry for version 1.96.21 by moving the note from the "Fixed" section to an
"Added" section and rewording the bullet to improve grammar; specifically
reference `searchExternalLocations` and state that the ability to configure it
via App Config was added (e.g., "Added ability to configure
`searchExternalLocations` via App Config.").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/map-template/src/components/MapTemplate/MapTemplate.jsx`:
- Around line 711-715: The effect that computes showExternalLocations only
treats the appConfig value as the string 'true', so native boolean true from
appConfig is ignored; update the logic in the useEffect that reads
externalLocationsConfig to handle both boolean and string forms (e.g., if
externalLocationsConfig is defined, set showExternalLocations to
(externalLocationsConfig === true || externalLocationsConfig === 'true'),
otherwise fall back to searchExternalLocations) and then call
setSearchExternalLocations(showExternalLocations); keep the useEffect
dependencies (searchExternalLocations, appConfig) and reference the existing
symbols useEffect, externalLocationsConfig, showExternalLocations,
setSearchExternalLocations, searchExternalLocations, and appConfig when making
the change.

---

Nitpick comments:
In `@packages/map-template/CHANGELOG.md`:
- Around line 8-12: Update the changelog entry for version 1.96.21 by moving the
note from the "Fixed" section to an "Added" section and rewording the bullet to
improve grammar; specifically reference `searchExternalLocations` and state that
the ability to configure it via App Config was added (e.g., "Added ability to
configure `searchExternalLocations` via App Config.").
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5284a129-b7c6-4599-8126-bb5704fe90d8

📥 Commits

Reviewing files that changed from the base of the PR and between 89b4f67 and b25f407.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • packages/map-template/CHANGELOG.md
  • packages/map-template/src/components/MapTemplate/MapTemplate.jsx

@matbmapspeople matbmapspeople merged commit fde9b19 into main Mar 31, 2026
1 check passed
@matbmapspeople matbmapspeople deleted the feature/matb/add_searchexternalid_appconfig branch March 31, 2026 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants