-
Notifications
You must be signed in to change notification settings - Fork 124
feat: extension model and form validation #2176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @nrjadkry, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust extension management system for MapStore within the application. It provides a new data model to store and track MapStore extensions, implements file upload capabilities with specific validation rules to ensure proper formatting of extension packages, and integrates a comprehensive administration interface for easy management and oversight of these extensions. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new Extension
model for managing MapStore extensions, complete with an admin interface and form validation for uploaded zip files. The changes are generally well-structured, but there is a significant issue where the extension's name
is not populated from the uploaded file, which will lead to errors. I have provided a fix for this. Additionally, I've included suggestions to improve code quality and performance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nrjadkry tests are required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nrjadkry let's wait for confirmation from @mattiagiupponi before applying proposed changes
|
||
urlpatterns += [ | ||
re_path("/client/extension", views.ExtensionsView.as_view(), name="mapstore-extension"), | ||
re_path("/client/pluginsconfig", views.PluginsConfigView.as_view(), name="mapstore-pluginsconfig"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested /client/pluginsconfig
locally but I'm seeing only the uploaded extension in the list, all the static plugins are missing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m currently using this path for the static plugins. Is this different from what it should be, @allyoucanmap?
base_config_path = os.path.join(
settings.PROJECT_ROOT, "static", "mapstore", "configs", "pluginsConfig.json"
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
geonode_mapstore_client/views.py
Outdated
"bundle": static(f"extensions/{ext.name}/index.js"), | ||
"translations": static(f"extensions/{ext.name}/translations"), | ||
"assets": static(f"extensions/{ext.name}/assets"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at the moment we have two variables telling the client the location of extensions, to make this work we should change following variables:
- MAPSTORE_EXTENSIONS_FOLDER_PATH in geonode settings.py
- EXTENSIONS_FOLDER_PATH in geonode-mapstore-client context_processor.py
now they are "/static/mapstore/extensions/"
but they should be an empty string ""
if we want to use absolute paths in this endpoint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I get your point @allyoucanmap, but I guess:
MAPSTORE_EXTENSIONS_FOLDER_PATH
(andEXTENSIONS_FOLDER_PATH
that gets its value from the same settings) are used by extensions bundled with the project. Right?MAPSTORE_EXTENSIONS_FOLDER_PATH
should be defined withSTATIC_URL
- the endpoints should use
MAPSTORE_EXTENSIONS_FOLDER_PATH
(orEXTENSIONS_FOLDER_PATH
) to ensure consistency
Am I wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- MAPSTORE_EXTENSIONS_FOLDER_PATH (and EXTENSIONS_FOLDER_PATH that gets its value from the same settings) are used by extensions bundled with the project. Right?
No, they represent the global location of all the extensions
- MAPSTORE_EXTENSIONS_FOLDER_PATH should be defined with STATIC_URL
- the endpoints should use MAPSTORE_EXTENSIONS_FOLDER_PATH (or EXTENSIONS_FOLDER_PATH) to ensure consistency
If we are doing this means MapStore will try to create the request with /static/mapstore/extensions/
(MAPSTORE_EXTENSIONS_FOLDER_PATH) + /static/mapstore/extensions/ExtensionName/index.js
(path created in the client/extensions endpoint ).
So if we want always to describe the absolute path inside the extensions configuration MAPSTORE_EXTENSIONS_FOLDER_PATH needs to be an empty string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got it now. The client already uses EXTENSIONS_FOLDER_PATH
from the geonode config.
As discussed @allyoucanmap we decide to keep the EXTENSIONS_FOLDER_PATH
as it is and this PR will be updated to:
- use the
EXTENSIONS_FOLDER_PATH
whenEXTENSIONS_STATIC_DIR
is used - the endpoints will use relative paths. i.e.
"bundle": "{ext.name}/index.js"),
"translations": "{ext.name}/translations"),
"assets": "{ext.name}/assets"),

Related Issue: #2177
Description
This pull request introduces a new feature for managing MapStore extensions directly from the GeoNode Django admin interface. It allows administrators to upload, update, configure, and delete extensions without needing to manually manage static files.
Key Changes
post_save
,post_delete
) to automatically unzip uploaded archives into the static directory and clean them up upon deletion.ExtensionAdminForm
) provides user-friendly validation to prevent uploading extensions with duplicate names.Screenshots
Django Admin View with the error message showing the duplicate name
