Skip to content
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

Improve/fix GEONODE_APPS_ENABLE handling #8712

Closed
etj opened this issue Jan 31, 2022 · 4 comments
Closed

Improve/fix GEONODE_APPS_ENABLE handling #8712

etj opened this issue Jan 31, 2022 · 4 comments
Assignees
Labels
3.2.x 3.3.x master regression Issues related to regressions.

Comments

@etj
Copy link
Contributor

etj commented Jan 31, 2022

Expected Behavior

When GEONODE_APPS_ENABLE is false, GeoApps should not be displayed.

Actual Behavior

Even when setting GEONODE_APPS_ENABLE to False, GeoApps options and link show up in the GUI.

Specifications

  • GeoNode version:
    • 3.3.x: happens here
    • master: not tested yet
@etj etj added this to the 3.3.1 milestone Jan 31, 2022
@giohappy giohappy added the 3.3.x label Jan 31, 2022
@giohappy giohappy removed this from the 3.3.1 milestone Jan 31, 2022
@giohappy giohappy added the regression Issues related to regressions. label Jan 31, 2022
@mattiagiupponi
Copy link
Contributor

By analyzing this issue with @etj we found out some issues or questions.
By using the GEONODE_APPS_ENABLE to exclude the geoapp from GeoNode works, but partially.

The problem is that even if we hide from the UI the GeoApp, all the other GeoApp functionality is still available:

  • Being able to use the API v1/v2 - Being able to call the apps/ URL and see the page
  • The GeoApp are still available in the resource Search (normal and advanced)

The first idea that we had (probably not the cleanest one) is to redirect all the GeoApp views to a 404 page to block the user to call those pages.
This approach works but is not 100% definite, because the client will always add the endpoint of the GeoApp in the v2

Here is available a brief preview of the work done till now.

We are open to suggestions and discussions about this approach. @giohappy @afabiani any thoughts?

@etj
Copy link
Contributor Author

etj commented Jan 31, 2022

Some more considerations: I do not expect this setting to be toggled very often.

ENABLE -> DISABLE transition:

This should be done only if geoapps were enable at first, in an environment where they are not needed.
Before changing the setting we could advise the admin to remove by hand all the geoapp instances created, so that they will not show up in the search.

DISABLE -> ENABLE transition:

Also this transition should be expected only once. I don't expect any problem in this transition.

@afabiani afabiani added the 3.2.x label Feb 2, 2022
@afabiani
Copy link
Member

afabiani commented Feb 2, 2022

@etj @mattiagiupponi in my opinion if someone disables the geoapps everything should be disabled too, and the model excluded also. Of course this will have some consequences. If someone tries to remove this after the apps has been initialized and/or some GeoApp has been created already, this might have some impact/consequences.

I agree with @etj if we describe very well on the settings section how to correctly use this parameter, the solution would be much cleaner.

@afabiani afabiani added the master label Feb 3, 2022
afabiani pushed a commit that referenced this issue Feb 4, 2022
* [Fixes #8712] Improve/fix GEONODE_APPS_ENABLE handling

* [Fixes #8712] Exclusion of geoapp from geonode urls if GEONODE_APPS_ENABLE is false
afabiani pushed a commit that referenced this issue Feb 4, 2022
* [Fixes #8712] Improve/fix GEONODE_APPS_ENABLE handling

* [Fixes #8712] Exclusion of geoapp from geonode urls if GEONODE_APPS_ENABLE is false

(cherry picked from commit a3b682d)

# Conflicts:
#	geonode/api/resourcebase_api.py
@afabiani
Copy link
Member

afabiani commented Feb 4, 2022

@etj PRs backported on 3.3.x and 3.2.x

@giohappy we need some clarifications in order to understand what this option actually is supposed to do on master branch, since the GeoApps are tightly bounded to che client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.2.x 3.3.x master regression Issues related to regressions.
Projects
None yet
Development

No branches or pull requests

4 participants