Missing notification for empty required fields in CorpusAudio, CorpusText.

[salva16]

In Corpus Audio info and Corpus Text info the Languages and Sizes components are shown in the 'Required' tab. However if nothing is entered in these components fields, no error is notified.

[jsteffen]

I think this is a general problem: It happens with all fields that are declared in the model with ..._model_set', REQUIRED ), I seems that the requirement is already fulfilled when there is a set, even if it's empty, but the the set should contain at least one item.

I bet this was also the actual reason for issue #256 about the empty audio size that was accepted. The class is defined like this in models.py:

class audioSizeInfoType_model(SchemaModel):
    """
    SizeInfo Element for Audio parts of a resource
    """

    class Meta:
        verbose_name = "Audio size"

    __schema_name__ = 'audioSizeInfoType'
    __schema_fields__ = (
      ( u'sizeInfo', u'sizeinfotype_model_set', REQUIRED ),
      ( u'durationOfEffectiveSpeechInfo', u'durationofeffectivespeechinfotype_model_set', OPTIONAL ),
      ( u'durationOfAudioInfo', u'durationofaudioinfotype_model_set', OPTIONAL ),
    )

Can a Django guru please have a look at this?

[salva16]

I think that the 'REQUIRED' option on *model_sets has no actual effect on validation. There should be the possibility to use the 'empty_permitted' flag on the first form of each formset declared as 'REQUIRED'. It could be worth to try it.

[jsteffen]

@ioannagian The problem is not solved completely yet. You cannot save a newly created resource with an empty set, but once you have added an item, you can EDIT the resource, remove the single item and save the resource again. When deleting the item, there's also a value error happening at command line. Maybe the change_view method in superadmin.py has to be adapted in the same way as the add_view method?

[salva16]

@ioannagian Yes, the fix should be applied also to change_view. It should also be checked, to be safer, which other classes in the metashare.repository.editor override add_view and/or change_view and apply the same fix.

[ioannagian]

Yes, it was also supposed to be in the change_view. Sorry, I was somehow confused with my different versions of superadmin. I am uploading the right file asap. @salva16 I did not find any other classes overriding these two functions, so this should be enough.

[cspurk]

@ioannagian There is still one case in which it doesn’t work; for example: When editing a valid corpusaudioinfotype_model and then deleting all language entries (using the “delete” checkbox in the upper right corner of the language entries), then the incomplete model can be saved. Could you please have a look at that and include this case in the unit tests? Thanks!

[salva16]

@ioannagian If it can be useful a Django form has a field named 'changed_data' that is a list containing the name of the fields that have been changed by the user. One special field is 'DELETE' that means that the user has marked the form for deletion. So instead of directly referring to forms[0] you could iterate forms[0] to forms[n-1], where n is the number of forms in the formset, and set the 'empty_permitted' flag on the first form that does not contain 'DELETE' in the 'changed_data' list.

[ioannagian]

@salva16 Thanks for the suggestion! I already tried something else: In the templates.admin.edit_inline.stacked.html, change the condition for displaying the delete checkbox(line 15) as follows:

From: if inline_admin_formset.formset.can_delete and inline_admin_form.original To : if inline_admin_formset.formset.can_delete and inline_admin_form.original and not forloop.first

Do you find this an acceptable solution?

[cspurk]

@ioannagian I don’t think this is a good solution as it would never allow a user to delete the first entry. This must be possible, though, either because there are already multiple entries or the user is just creating a new one but would like to remove the old one.

[cspurk]

Fixed with pull request #457.