Form fields with 'switched' can't be set required=True

Bug #1755131 reported by Wangliangyu
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Dashboard (Horizon)
Fix Released
Medium
Akihiro Motoki

Bug Description

Some form fields like class AttachInterface in dashboards/project/instances/forms.py:

class AttachInterface(forms.SelfHandlingForm):
    specification_method = forms.ThemableChoiceField(
        label=_("The way to specify an interface"),
        initial=False,
        widget=forms.ThemableSelectWidget(attrs={
            'class': 'switchable',
            'data-slug': 'specification_method',
        }))
    network = forms.ThemableChoiceField(
        label=_("Network"),
        required=False,
        widget=forms.ThemableSelectWidget(attrs={
            'class': 'switched',
            'data-switch-on': 'specification_method',
            'data-specification_method-network': _('Network'),
        }))

When the value of specification_method field is selected as network,the network filed is necessary and should
set required=True.But when the value is selected as port, the network field is not necessary and the network
field is also be checked and return an error.Now, we need the star when the network is necessary and ignore the
check when it is not necessary.

Wangliangyu (wangly)
Changed in horizon:
assignee: nobody → Wangliangyu (wangly)
Revision history for this message
Akihiro Motoki (amotoki) wrote :

> Now, we need the star when the network is necessary and ignore the
check when it is not necessary.

'required' field in a Django form is checked one by one.
IMHO what we need to do is to explore a way to declare "conditionally required" (to show an asterisk mark) rather than skipping the check conditionally. This is because 'required' comes from Django but an asterisk mark is introduced by horizon. We should not change the django behavior of 'required' check. It is better to explore a way to show an asterisk mark for "swit hed" conditional field (with required=False).

Revision history for this message
Akihiro Motoki (amotoki) wrote :

bug 1420370 reports the same problem.

Changed in horizon:
importance: Undecided → Medium
status: New → Confirmed
Revision history for this message
Wangliangyu (wangly) wrote :

> We should not change the django behavior of 'required' check. It is better to explore a way to show an asterisk mark for "swit hed" conditional field (with required=False).

If just show an asterisk mark, we need additional field clean method, like method 'clean_network'.
So, my initial idea was skipping the check or removing the error conditionally.
I will submit a patch first and consider your opinion.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to horizon (master)

Fix proposed to branch: master
Review: https://review.openstack.org/552247

Changed in horizon:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to horizon (master)

Reviewed: https://review.openstack.org/552247
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=0efbb377a5713ec49d13152b09544412ffe7eeed
Submitter: Zuul
Branch: master

commit 0efbb377a5713ec49d13152b09544412ffe7eeed
Author: wangliangyu <email address hidden>
Date: Tue Mar 13 10:38:32 2018 +0800

    Show an asterisk mark for 'switched' conditional filed(required=Flase)

    Change-Id: I80dbd4b6aa9f3adeaf508b05f878b9f11cacf80a
    Partial-Bug: #1755131

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to horizon (master)

Fix proposed to branch: master
Review: https://review.openstack.org/555096

Changed in horizon:
assignee: Wangliangyu (wangly) → Akihiro Motoki (amotoki)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to horizon (master)

Reviewed: https://review.openstack.org/555096
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=f8196331ea139f52b303eb87d0280152f06d9475
Submitter: Zuul
Branch: master

commit f8196331ea139f52b303eb87d0280152f06d9475
Author: Akihiro Motoki <email address hidden>
Date: Thu Mar 22 08:32:00 2018 +0900

    Add asterisk to conditionally required fields

    https://review.openstack.org/#/c/552247/ introduced a way to
    add asterisk mark to conditionally required fields.

    This commit covers several known forms:
    - Create Network / Create Subnet
    - Create Port
    - Attach Interface to Instance
    - Add Security Group Rule

    Change-Id: I809c23fd64dc2f379c3fdb585741c6b266ec5b1b
    Closes-Bug: #1420370
    Closes-Bug: #1292165
    Closes-Bug: #1755131

Changed in horizon:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/horizon 14.0.0.0b1

This issue was fixed in the openstack/horizon 14.0.0.0b1 development milestone.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.