[Fixes #14519] GroupProfile.access default is "public'", not a valid choice - #14520
[Fixes #14519] GroupProfile.access default is "public'", not a valid choice#14520christianbraun wants to merge 1 commit into
Conversation
… valid choice The default carried a stray apostrophe, so it matched none of the field's own GROUP_CHOICES. Any GroupProfile created without an explicit access — which is what geonode_ldap's group mirroring does — was stored as "public'" and was therefore invisible to code filtering on access="public", such as geonode.people.utils.get_available_users. Present since 796add4 (2014). Adds a data migration so existing rows are repaired too; fixing the default alone would leave them broken, and fixing only the data would let the next group sync write the bad value straight back.
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @christianbraun on file. In order for us to review and merge your code, please contact the project maintainers to get yourself added. |
|
The CLA is signed and sent. As the bot notes, @christianbraun still needs adding on your side — could a maintainer take care of that? Meanwhile |
|
Thanks @christianbraun for this long-stainding silent bug. |
Fixes #14519.
GroupProfile.accesswas declared withdefault="public'"— a stray apostrophe, matching none of the field's ownGROUP_CHOICES. Any group created without an explicitaccesswas stored with that value and became invisible to code filtering onaccess="public", notablygeonode.people.utils.get_available_users. The typo has been there since 796add4 (2014) and is also in24_initial.py.Three changes:
geonode/groups/models.py—default="public'"→default="public".geonode/groups/migrations/0036_fix_groupprofile_access_default.py— theAlterFieldfor the new default, plus aRunPythonthat rewrites existing"public'"rows to"public". Existing installs need the data fix; the model fix alone leaves every group already created still broken. TheRunPythonisnoopin reverse, since undoing it would mean writing an invalid value back.geonode/groups/tests.py—test_default_access_is_a_valid_choice, asserting aGroupProfilecreated without anaccessis both a valid choice and returned byfilter(access="public").How we hit it: on an AD-backed instance,
geonode_ldap'supdateldapgroupscreatesGroupProfiles without passingaccess, so all 276 mirrored groups carried"public'"and a group picker filtering on"public"showed none of them.Most of core is unaffected because it keys on
access == "private"instead, which the typo leaves alone — that is why this has survived so long.Checklist
For all pull requests:
The following are required only for core and extension modules (they are welcomed, but not required, for contrib modules):
<issue_number>] Title of the PR"Happy to backport to 4.4.x / 5.0.x once this lands. I could not run the full suite locally, so CI is the first real execution of the migration.