openwisp
[fix] Select automatically organization when only 1 org #170
Coverage increased (+0.3%) to 97.584% when pulling 6b1c118a2153c48f23a6d3020b392d5a0116207c on niteshsinha17:issues/170-automatic-org-selection-on-1-org into 114b16b51a415223cb2f34fd2a753bfe6a35d27f on openwisp:master.
@NoumbissiValere I don't have any idea of writing tests. But i will try to learn and understand it through the code after that i will add tests for it as soon as possible.
@NoumbissiValere I don't have any idea of writing tests. But i will try to learn and understand it through the code after that i will add tests for it as soon as possible.
@niteshsinha17 you could also get help from django testing docs
@niteshsinha17 sorry I forgot to add my review last time I tested this, I didn't see the organization being selected automatically when the user had only one org, could you share a gif of the result you get please?
Sure, I have shared it.
I think cache is store in your system. You may need to create or delete an organization to change it.
@nemesisdesign Thanks for the review.
I've tested it again and found the issue.
I tried to replicate this but it was not occurring in my system. During this, I found one more Issue and I have solved it. I think it may have also solved the issue about which you are talking.
This would radically simplify the solution and reduce the amount of lines required. What do you think? I think we need to do the same thing during the creation of a user and it's not going to work for it. We can modify
SelectOrgMixinto simplify our work for both cases.
class SelectOrgMixin(object):
def select_organization(self, request, form):
'''
When the user has access to only one organization
automatically set that org as the default value
'''
if request.method == 'POST':
return
fields = form.base_fields
if 'organization' in fields:
org_field = fields['organization']
org_field.initial = request.user.selected_org(org_field)
org_field.empty_label = self._get_empty_level(
request.user.is_superuser, org_field
)
def _get_empty_level(self, superuser, org_field):
if superuser:
if not org_field.required:
return _('Shared systemwide (no organization)')
elif org_field._queryset.count() == 1:
return None
return org_field.empty_label
return None
Then we can remove these lines too. https://github.com/openwisp/openwisp-users/blob/a23e05fee407f739803740a0e800fc1fa3ad33f4/openwisp_users/multitenancy.py#L70-L71 and https://github.com/openwisp/openwisp-users/blob/a23e05fee407f739803740a0e800fc1fa3ad33f4/openwisp_users/multitenancy.py#L78 What do you think?
Also,
When object can be shared between multiple organizations and only one organization is present then which should be selected as default?
This case is happening with the Template model and tests are failing at these lines.
https://github.com/openwisp/openwisp-users/blob/a23e05fee407f739803740a0e800fc1fa3ad33f4/openwisp_users/tests/test_admin.py#L1391-L1395
Is it easy to understand or I should push the code before making final changes?
@nemesisdesign It is ready for review. Do I need to resolve this conflict by rebasing??
@nemesisdesign It is ready for review. Do I need to resolve this conflict by rebasing??
Yes @niteshsinha17 please solve the merge conflicts
Re-based to current master. Ready for review.
I have one doubt. When org field is not required then we are setting empty label to Shared systemwide (no organization). but when there is only one organization and the org field is not required which should be chosen as initial value? Currently Shared systemwide (no organization) is shown as selected value because it is an empty label. @nemesisdesign Can you please answer this.
@niteshsinha17 may be the only organization should be selected when there is only one organization. 🤔
@niteshsinha17 may be the only organization should be selected when there is only one organization. 🤔
Actually, there is only one org but it looks like has 2 org because of the empty label and empty label seems to be selected indeed it has not been selected. I think it is quite complicated. We should just ignore it for this case otherwise we will need to change lots of things.
Superseded by https://github.com/openwisp/openwisp-users/issues/333, thanks to all who contributed to this effort :pray: .
