Ocultando etiqueta #9
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements functionality to hide/customize the dashboard label by making the dashboard title configurable through CKAN configuration. The changes allow administrators to customize the dashboard section title instead of using a hardcoded "Dashboard" label.
- Adds a new helper function to retrieve dashboard title from configuration
- Makes the dashboard title display conditional and configurable
- Updates the template to use the configurable title and conditionally show description
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| ckanext/dashboard/templates/package/read.html | Updates template to use configurable dashboard title and conditionally display description |
| ckanext/dashboard/plugin.py | Adds configuration for dashboard title and registers new helper function |
| ckanext/dashboard/helpers.py | Implements helper function to retrieve dashboard title from configuration |
ckanext/dashboard/helpers.py
Outdated
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Using a bare except Exception: is too broad and can mask unexpected errors. Consider catching specific exceptions like KeyError, AttributeError, or ValidationError that might be raised by the config_option_show action.
| except Exception: | |
| pass | |
| except (KeyError, AttributeError, t.ValidationError) as e: | |
| t.log.error(f"Error retrieving dashboard title from config: {e}") |
Copilot uses AI. Check for mistakes.
ckanext/dashboard/helpers.py
Outdated
|
|
||
|
|
||
| def get_dashboard_title_from_config(): | ||
| """Obtiene el título del dashboard desde la configuración del .ini""" |
There was a problem hiding this comment.
[nitpick] The docstring is in Spanish while the codebase appears to use English for documentation. Consider using English for consistency: 'Gets the dashboard title from the .ini configuration'.
| """Obtiene el título del dashboard desde la configuración del .ini""" | |
| """Gets the dashboard title from the .ini configuration""" |
Copilot uses AI. Check for mistakes.
ckanext/dashboard/plugin.py
Outdated
| @@ -25,6 +25,10 @@ def update_config(self, config_): | |||
| toolkit.add_template_directory(config_, "templates") | |||
| toolkit.add_public_directory(config_, "public") | |||
| toolkit.add_resource("assets", "dashboard") | |||
| # Opción para personalizar el título del dashboard | |||
There was a problem hiding this comment.
[nitpick] The comment is in Spanish while the codebase appears to use English for documentation. Consider using English for consistency: '# Option to customize the dashboard title'.
| # Opción para personalizar el título del dashboard | |
| # Option to customize the dashboard title |
Copilot uses AI. Check for mistakes.
ckanext/dashboard/plugin.py
Outdated
| return { | ||
| 'get_dataset_dashboard': h.get_dataset_dashboard, | ||
| 'get_dashboard_title_from_config': h.get_dashboard_title_from_config | ||
| } | ||
|
|
||
| def i18n_locales(self): | ||
| """Lanaguages this plugin has translations for.""" |
There was a problem hiding this comment.
There's a typo in the docstring: 'Lanaguages' should be 'Languages'.
| """Lanaguages this plugin has translations for.""" | |
| """Languages this plugin has translations for.""" |
Copilot uses AI. Check for mistakes.
| @@ -25,6 +25,10 @@ def update_config(self, config_): | |||
| toolkit.add_template_directory(config_, "templates") | |||
| toolkit.add_public_directory(config_, "public") | |||
| toolkit.add_resource("assets", "dashboard") | |||
| # Option to customize the dashboard title | |||
| config_['ckanext.bcie.dashboard_title'] = config_.get( | |||
| 'ckanext.bcie.dashboard_title', 'Tablero' | |||
There was a problem hiding this comment.
| 'ckanext.bcie.dashboard_title', 'Tablero' | |
| 'ckanext.bcie.dashboard_title', 'Dashboard' |
Default in english for this extension
| def get_dashboard_title_from_config(): | ||
| """Gets the dashboard title from the .ini configuration""" | ||
| try: | ||
| result = t.get_action('config_option_show')( |
There was a problem hiding this comment.
This action do not exist
You just need to get the config option from config.get('ckanext.bcie.dashboard_title')
Check examples in other extensions
| @@ -25,6 +25,10 @@ def update_config(self, config_): | |||
| toolkit.add_template_directory(config_, "templates") | |||
| toolkit.add_public_directory(config_, "public") | |||
| toolkit.add_resource("assets", "dashboard") | |||
| # Option to customize the dashboard title | |||
| config_['ckanext.bcie.dashboard_title'] = config_.get( | |||
There was a problem hiding this comment.
Here we usually define the config validator to use later in sysadmin config form.
This is not required here as this.
Check other extensions usage
|
Add a reference in README.md for this new config |
Allow changing the dashboard title with a sysadmin config option