mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 13:47:19 +08:00
fix: address review comments for marketplace_path setting
- Fix Data Model Inconsistency: Change backend default from 'marketplaces/default.json' to None. Now null means 'load all skills without filtering', and any string value means use that marketplace. - Add Alembic migration: Create migration 099 to add marketplace_path column to user_settings table for enterprise deployments. - Simplify frontend handling: Use parseMarketplacePath() utility to consistently convert empty string to null. Remove confusing DEFAULT_MARKETPLACE_PATH constant, use MARKETPLACE_PATH_PLACEHOLDER for UI hint only. - Add validation: Implement isValidMarketplacePath() to validate marketplace path formats (simple paths like 'marketplaces/default.json' or cross-repo paths like 'owner/repo:path/file.json'). Show validation errors inline and disable save button for invalid input. - Add tests: Frontend tests for isValidMarketplacePath() and parseMarketplacePath(). Backend tests for marketplace_path default and custom value handling. - Add i18n: Translation for marketplace path validation error message. Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
@@ -127,3 +127,21 @@ def test_settings_no_pydantic_frozen_field_warning():
|
||||
assert len(frozen_warnings) == 0, (
|
||||
f'Pydantic frozen field warnings found: {[str(w.message) for w in frozen_warnings]}'
|
||||
)
|
||||
|
||||
|
||||
def test_settings_marketplace_path_default():
|
||||
"""Test that marketplace_path defaults to None (load all skills)."""
|
||||
settings = Settings()
|
||||
assert settings.marketplace_path is None
|
||||
|
||||
|
||||
def test_settings_marketplace_path_custom():
|
||||
"""Test that marketplace_path can be set to a custom value."""
|
||||
settings = Settings(marketplace_path='marketplaces/custom.json')
|
||||
assert settings.marketplace_path == 'marketplaces/custom.json'
|
||||
|
||||
|
||||
def test_settings_marketplace_path_none_explicit():
|
||||
"""Test that marketplace_path can be explicitly set to None."""
|
||||
settings = Settings(marketplace_path=None)
|
||||
assert settings.marketplace_path is None
|
||||
|
||||
Reference in New Issue
Block a user