Add a form for working with key/value stores instead of models.

Review Request #7853 — Created Jan. 7, 2016 and submitted

Information

Djblets
release-0.10.x

Reviewers

Forms generally are bound to models, or are completely custom and devoid
of any real default logic beyond rendering and validation. We have some
forms in Djblets that interface with models or other objects, treating
them like key/value stores using set and get methods. These are
all based on SiteSettingsForm, which is built for SiteConfiguration,
and isn't always the best model to use.

This change introduces a new form, KeyValueForm, which works as a base
for all these other forms, SiteSettingsForm included. It handles the
loading of data from an object, the saving of form data back out to
the object, and load/save blacklists for fields (a feature used by
these other forms). It also handles easily marking some fields as
disabled, another feature used by these other forms.

It's built to be usable with any type of key/value storage backend,
by providing several functions that can be overridden to handle the
getting and setting of keys and their values and the creation/saving
of instances.

Unit tests pass in both Djblets and Review Board.

Manually tested configuring extensions and saving the various site
settings forms.

Description From Last Updated

Instead of "list or tuple", how about "iterable"?

daviddavid

The final clause in this sentence is superfluous (you already said "the values from that instance").

daviddavid

status, not statuses

daviddavid

Use six.iteritems?

daviddavid

Typo: cange

daviddavid
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/extensions/forms.py
        djblets/forms/tests/test_key_value_form.py
        djblets/forms/forms/key_value_form.py
        djblets/siteconfig/forms.py
        djblets/forms/forms/__init__.py
    
    Ignored Files:
        djblets/forms/tests/__init__.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/extensions/forms.py
        djblets/forms/tests/test_key_value_form.py
        djblets/forms/forms/key_value_form.py
        djblets/siteconfig/forms.py
        djblets/forms/forms/__init__.py
    
    Ignored Files:
        djblets/forms/tests/__init__.py
    
    
  2. 
      
david
  1. In your description, "void of" should be "devoid of"

  2. djblets/forms/forms/key_value_form.py (Diff revision 1)
     
     
    Show all issues

    Instead of "list or tuple", how about "iterable"?

    1. I'm actually just going to go with "list", as in a list of things, not necessarily the type. I think it's clear enough, and is what the Django docs do anyway.

  3. djblets/forms/forms/key_value_form.py (Diff revision 1)
     
     
     
     
    Show all issues

    The final clause in this sentence is superfluous (you already said "the values from that instance").

  4. djblets/forms/forms/key_value_form.py (Diff revision 1)
     
     
    Show all issues

    status, not statuses

  5. djblets/forms/forms/key_value_form.py (Diff revision 1)
     
     
     
    Show all issues

    Use six.iteritems?

    1. six.iterkeys. For what it's worth, it's legal to iterate over a dictionary. You'll just get the keys. Switching anyway to make it more clear.

    2. What I mean is that you're fetching both the key and the value, so you could just do:

      for field_name, field in six.iteritems(self.fields):
      
    3. Oh. Right. Silly me.

  6. djblets/forms/forms/key_value_form.py (Diff revision 1)
     
     
    Show all issues

    Typo: cange

  7. 
      
chipx86
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/extensions/forms.py
        djblets/forms/tests/test_key_value_form.py
        djblets/forms/forms/key_value_form.py
        djblets/siteconfig/forms.py
        djblets/forms/forms/__init__.py
    
    Ignored Files:
        djblets/forms/tests/__init__.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/extensions/forms.py
        djblets/forms/tests/test_key_value_form.py
        djblets/forms/forms/key_value_form.py
        djblets/siteconfig/forms.py
        djblets/forms/forms/__init__.py
    
    Ignored Files:
        djblets/forms/tests/__init__.py
    
    
  2. 
      
chipx86
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/extensions/forms.py
        djblets/forms/tests/test_key_value_form.py
        djblets/forms/forms/key_value_form.py
        djblets/siteconfig/forms.py
        djblets/forms/forms/__init__.py
    
    Ignored Files:
        djblets/forms/tests/__init__.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/extensions/forms.py
        djblets/forms/tests/test_key_value_form.py
        djblets/forms/forms/key_value_form.py
        djblets/siteconfig/forms.py
        djblets/forms/forms/__init__.py
    
    Ignored Files:
        djblets/forms/tests/__init__.py
    
    
  2. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-0.10.x (dc7e111)
GU
  1. my first review

    1. This isn't a demo server. Please don't post tests here. You can use http://demo.reviewboard.org/ for this instead.

  2. 
      
GU
  1. ok

  2.