Some fields in testplans/forms.py are not used

[atodorov]

#655 removes labels from form fields since we prefer having the text in the HTML templates. However In NewPlanForm there is the tag field which doesn't seem like it is in use.

Definitely not in use when creating a new TP or editting existing TP. However there are inherited form which may be using this field.

[atodorov]

same goes for EditPlanForm.author

[asankov]

After investigating this here is what I found:

NewPlanForm.tag and EditPlanForm.author are not used anywhere, including in the forms that inherit them, so it looks like they can be safely deleted.

Moreover, when looking further into the classes that inheret these two I found out more things we can improve:

• In testplans/forms.py there is this piece of code:

class XMLRPCNewPlanForm(EditPlanForm):
    text = forms.CharField()

class XMLRPCEditPlanForm(NewPlanForm):
    name = forms.CharField(
        label="Plan name", required=False
    )
    type = forms.ModelChoiceField(
        label="Type",
        queryset=PlanType.objects.all(),
        required=False
    )
    product = forms.ModelChoiceField(
        label="Product",
        queryset=Product.objects.all(),
        required=False,
    )
    product_version = forms.ModelChoiceField(
        label="Product Version",
        queryset=Version.objects.none(),
        required=False
    )

These two form - XMLRPCNewPlanForm and XMLRPCEditPlanForm are not used anywhere, but are inherited byNewPlanForm and EditPlanForm in xmlrpc/forms.py. IMO we can delete them by moving their fields into the child classes

Also this raises the questions - Why do we have forms that deal with XMLRPC in testplans/forms.py when we have a separate package form xmlrpc stuff - xmlrpc?

Moreover, we support both XML-RPC and JSON-RPC and AFAIK they both use the same code base, just the parser is different. So the JSON-RPC API uses a lot of classes that are prefixed with XMLRPC or are in the xmlrpc package. How about we omit XML everywhere in favour of just RPC? Or we may not prefix them with anything since they will be xmlrpc/rpc package?

Here is what needs to be done IMO:

• delete NewPlanForm.tag and EditPlanForm.author since they are not used
• delete classes that are not used, but are only inherited once - XMLRPCNewPlanForm, XMLRPCEditPlanForm
• rename all XMLRPC* to RPC*
• move all of the RPC* classes to rpc package
• investigate all of the above form classes for unused fields

@atodorov what do you think about this plan? please give your opinion and I can start working on this immediately

[atodorov]

delete NewPlanForm.tag and EditPlanForm.author since they are not used

+1, you can go ahead and delete the entire EditPlanForm class and change all of the inherited classes to inherit from NewPlanForm. Use separate commits when changing the two forms.

delete classes that are not used, but are only inherited once - XMLRPCNewPlanForm, XMLRPCEditPlanForm

Be careful here!!! These are not "not used". They are used as a base for other classes, which are then used when handling RPC requests, so they are very much in use. The correct form of action here is to:

• copy the fields from these intermediate classes inside the ones found under xmlrpc/forms.py
• remove the intermediate classes
• under xmlrpc/ better organize the forms into smaller groups, e.g. xmrpc/api/forms/testplan.py or similar and update the imports in the API modules.

investigate all of the above form classes for unused fields

this is part of the change above (separate commits for everything) and you should be careful for:

• all of these inherited forms redefine class fields and from what I've seen in practice many times there are duplicates. The only reason for redefining some field is if you want to make it required/not required or change some defaults, etc. which will be different between webUI and RPC. There's no other reason for these redefined fields to exist.
• Some of these fields are not properly covered by tests and my approach in the rare event I had removed some was to inspect every field one by one, inspect what tests are there (and what is validated inside of them) and then manually try both the UI and the API to make sure it all works.

move all of the RPC* classes to rpc package

Not now, open a separate issue for this. This is a rather big change which needs to be planned and accounted for. I would rather spend the time to refactor these as described above instead of just moving the around.

rename all XMLRPC* to RPC*

This is renaming only and making sure everything matches. We can rename the module itself easily but I prefer to update the classes and functions one-by-one to avoid massive changes.

However the naming is only due to historical reasons and you are welcome to implement a pylint checker that will warn us about bogus names (and also drive the rename/refactor effort). Again new issue so we can plan this.

Also this raises the questions - Why do we have forms that deal with XMLRPC in testplans/forms.py when we have a separate package form xmlrpc stuff - xmlrpc?

To answer this - legacy!