Re: [openstack-dev] [Horizon] Edit subnet in workflows - ip_version hidden?

2014-03-17 Thread Akihiro Motoki
Hi Abishek, Radomir,

I just noticed this mail.

It seems better the code discussed will be refactored.
UpdateSubnetInfoAction in projects/networks/subnets/workflow.py
inherits CreateSubnetInfoAction in projects/networks/workflow.py.
IIRC I would like to share most logic between two and tried to
remove ip_version from the parent class (in networks.workflow)
in the child class and the current implemention just worked.
It is no more than it.

After looking at it now, it looks enough just to delete ip_version
from self.fields. The same thing can say 'with_subnet' attribute
in networks.subnets.workflows.CreateSubnetInfoAction

When I implemented this, I was not so familir with Django
and perhaps I must have not know deleting a field from self.fields :-(
Thanks for raising this!

Akihiro

(2014/03/12 15:43), Radomir Dopieralski wrote:
 On 11/03/14 16:57, Abishek Subramanian (absubram) wrote:

 Althouh - how up to date is this code?

 This should be easy to check with the git blame command:

 $ git blame
 openstack_dashboard/dashboards/project/networks/subnets/workflows.py

 [...]
 31d55e50 (Akihiro MOTOKI  2013-01-04 18:33:03 +0900  56) class
 CreateSubnet(network_workflows.CreateNetwork):
 [...]
 31d55e50 (Akihiro MOTOKI  2013-01-04 18:33:03 +0900  82) class
 UpdateSubnetInfoAction(CreateSubnetInfoAction):
 [...]
 31d55e50 (Akihiro MOTOKI  2013-01-04 18:33:03 +0900 101)
  #widget=forms.Select(
 [...]

 As you can see, it's all in the same patch, so it's on purpose.

 It seems to me, that in the update dialog you are not supposed to change
 the IP Version field, Akihiro Motoki tried to disable it
 first, but then he hit the problem with the browser not submitting
 the field's value and the form displaying the wrong option in there,
 so he decided to hide it instead. But we won't know until the author
 speaks for himself.

 Personally, I would also add a check in the clean() method that the
 IP Version field value indeed didn't change -- to make sure nobody
 edited the form's HTML to get rid of the disabled or readonly attribute.

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [Horizon] Edit subnet in workflows - ip_version hidden?

2014-03-12 Thread Radomir Dopieralski
On 11/03/14 16:57, Abishek Subramanian (absubram) wrote:

 Althouh - how up to date is this code?

This should be easy to check with the git blame command:

$ git blame
openstack_dashboard/dashboards/project/networks/subnets/workflows.py

[...]
31d55e50 (Akihiro MOTOKI  2013-01-04 18:33:03 +0900  56) class
CreateSubnet(network_workflows.CreateNetwork):
[...]
31d55e50 (Akihiro MOTOKI  2013-01-04 18:33:03 +0900  82) class
UpdateSubnetInfoAction(CreateSubnetInfoAction):
[...]
31d55e50 (Akihiro MOTOKI  2013-01-04 18:33:03 +0900 101)
#widget=forms.Select(
[...]

As you can see, it's all in the same patch, so it's on purpose.

It seems to me, that in the update dialog you are not supposed to change
the IP Version field, Akihiro Motoki tried to disable it
first, but then he hit the problem with the browser not submitting
the field's value and the form displaying the wrong option in there,
so he decided to hide it instead. But we won't know until the author
speaks for himself.

Personally, I would also add a check in the clean() method that the
IP Version field value indeed didn't change -- to make sure nobody
edited the form's HTML to get rid of the disabled or readonly attribute.

-- 
Radomir Dopieralski

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


[openstack-dev] [Horizon] Edit subnet in workflows - ip_version hidden?

2014-03-11 Thread Abishek Subramanian (absubram)
Hi,

I had a question regarding the
dashboards/project/networks/subnets/workflows.py
file and in particular the portion of the ip_version field.

It is marked as a hidden input field for the update subnet class with this
note.

# NOTE(amotoki): When 'disabled' attribute is set for the ChoiceField
# and ValidationError is raised for POST request, the initial value of
# the ip_version ChoiceField is not set in the re-displayed form
# As a result, 'IPv4' is displayed even when IPv6 is used if
# ValidationError is detected. In addition 'required=True' check
complains
# when re-POST since the value of the ChoiceField is not set.
# Thus now I use HiddenInput for the ip_version ChoiceField as a work
# around.



Can I get a little more context to this please?
I'm not sure I understand why it says this field always is displayed as
IPv4.
Is this still the case? Adding some debug logs I seem to see that the
ipversion is correctly being detected as 4 or 6 as the case may be.

Thanks!



___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [Horizon] Edit subnet in workflows - ip_version hidden?

2014-03-11 Thread Radomir Dopieralski
On 11/03/14 15:52, Abishek Subramanian (absubram) wrote:
 Hi,
 
 I had a question regarding the
 dashboards/project/networks/subnets/workflows.py
 file and in particular the portion of the ip_version field.
 
 It is marked as a hidden input field for the update subnet class with this
 note.
 
 # NOTE(amotoki): When 'disabled' attribute is set for the ChoiceField
 # and ValidationError is raised for POST request, the initial value of
 # the ip_version ChoiceField is not set in the re-displayed form
 # As a result, 'IPv4' is displayed even when IPv6 is used if
 # ValidationError is detected. In addition 'required=True' check
 complains
 # when re-POST since the value of the ChoiceField is not set.
 # Thus now I use HiddenInput for the ip_version ChoiceField as a work
 # around.
 
 
 
 Can I get a little more context to this please?
 I'm not sure I understand why it says this field always is displayed as
 IPv4.
 Is this still the case? Adding some debug logs I seem to see that the
 ipversion is correctly being detected as 4 or 6 as the case may be.

Some browsers (Chrome, iirc) will not submit the values from form fields
that are disabled. That means, that when re-displaying this form
(after an error in any other field, for example), that field's value
will be missing, and the browser will happily display the first option,
which is ipv4.

Another solution could be perhaps using readonly instead of disabled.

-- 
Radomir Dopieralski


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [Horizon] Edit subnet in workflows - ip_version hidden?

2014-03-11 Thread Abishek Subramanian (absubram)
Thanks Radomir.

Yes I've changed it to a readonly. But just wanted to double check
I didn't end up breaking something elsewhere :)

Althouh - how up to date is this code?

These are the actual lines of code -

# NOTE(amotoki): When 'disabled' attribute is set for the ChoiceField
# and ValidationError is raised for POST request, the initial value of
# the ip_version ChoiceField is not set in the re-displayed form
# As a result, 'IPv4' is displayed even when IPv6 is used if
# ValidationError is detected. In addition 'required=True' check
complains
# when re-POST since the value of the ChoiceField is not set.
# Thus now I use HiddenInput for the ip_version ChoiceField as a work
# around.
ip_version = forms.ChoiceField(choices=[(4, 'IPv4'), (6, 'IPv6')],
   #widget=forms.Select(
   #attrs={'disabled': 'disabled'}),
   widget=forms.HiddenInput(),
   label=_(IP Version))




I don't think ip_version even has an attribute or an option to be set to
'disabled'.
Is this from an old version where the create side got fixed but the update
side was forgotten about?


On 3/11/14 11:30 AM, Radomir Dopieralski openst...@sheep.art.pl wrote:

On 11/03/14 15:52, Abishek Subramanian (absubram) wrote:
 Hi,
 
 I had a question regarding the
 dashboards/project/networks/subnets/workflows.py
 file and in particular the portion of the ip_version field.
 
 It is marked as a hidden input field for the update subnet class with
this
 note.
 
 # NOTE(amotoki): When 'disabled' attribute is set for the ChoiceField
 # and ValidationError is raised for POST request, the initial value
of
 # the ip_version ChoiceField is not set in the re-displayed form
 # As a result, 'IPv4' is displayed even when IPv6 is used if
 # ValidationError is detected. In addition 'required=True' check
 complains
 # when re-POST since the value of the ChoiceField is not set.
 # Thus now I use HiddenInput for the ip_version ChoiceField as a
work
 # around.
 
 
 
 Can I get a little more context to this please?
 I'm not sure I understand why it says this field always is displayed as
 IPv4.
 Is this still the case? Adding some debug logs I seem to see that the
 ipversion is correctly being detected as 4 or 6 as the case may be.

Some browsers (Chrome, iirc) will not submit the values from form fields
that are disabled. That means, that when re-displaying this form
(after an error in any other field, for example), that field's value
will be missing, and the browser will happily display the first option,
which is ipv4.

Another solution could be perhaps using readonly instead of disabled.

-- 
Radomir Dopieralski


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev