-
Notifications
You must be signed in to change notification settings - Fork 0
Mvdb/jkr 9392/fix listifield #8
Changes from all commits
88fd139
157f990
538a306
6c4989c
3679d98
59dbdea
79869b5
cc3be55
494fa4f
24f4e74
9a0f922
6647375
4b5886d
3afdf8e
53154a7
6dbc59b
1a42aef
d007978
87e2f7b
ea49109
864cfc1
235eba2
c86d3cb
e005f82
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -116,7 +116,21 @@ def construct_instance(form, instance, fields=None, exclude=None): | |
| elif isinstance(f, ListField): | ||
| list_field = getattr(instance, f.name) | ||
| uploads = cleaned_data[f.name] | ||
| idx_to_pop = [] | ||
| for i, uploaded_file in enumerate(uploads): | ||
| if isinstance(uploaded_file, list): # ListOfFilesWidget | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you test instead on
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes indeed |
||
| uploaded_file, to_delete = uploaded_file | ||
| if to_delete: | ||
| try: | ||
| list_field[i].delete() | ||
| idx_to_pop.append(i) | ||
| except IndexError: | ||
| # someone checked the delete box of the last | ||
| # form item, which obviously doesnt exist | ||
| # on the list | ||
| pass | ||
| continue | ||
|
|
||
| if uploaded_file is None: | ||
| continue | ||
| try: | ||
|
|
@@ -129,6 +143,10 @@ def construct_instance(form, instance, fields=None, exclude=None): | |
| list_field[i] = file_obj | ||
| except IndexError: | ||
| list_field.append(file_obj) | ||
|
|
||
| for idx in reversed(idx_to_pop): | ||
| del list_field[idx] | ||
|
|
||
| setattr(instance, f.name, list_field) | ||
| else: | ||
| field = getattr(instance, f.name) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,7 +32,7 @@ | |
| except ImportError: | ||
| from pymongo.errors import InvalidId | ||
|
|
||
| from mongodbforms.widgets import ListWidget, MapWidget, HiddenMapWidget | ||
| from mongodbforms.widgets import ListWidget, MapWidget, HiddenMapWidget, ListOfFilesWidget | ||
|
|
||
|
|
||
| class MongoChoiceIterator(object): | ||
|
|
@@ -398,3 +398,38 @@ def _has_changed(self, initial, data): | |
| if self.contained_field._has_changed(init_val, v): | ||
| return True | ||
| return False | ||
|
|
||
|
|
||
| class ListOfFilesField(ListField): | ||
| widget = ListOfFilesWidget | ||
|
|
||
| def clean(self, value): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Docstring? Complex method, worth summarizing the things it'll check. |
||
| """ | ||
| We clean every subwidget. | ||
| """ | ||
| clean_data = [] | ||
| errors = ErrorList() | ||
| is_empty = not value or not [v for v in value if v not in self.empty_values] | ||
| if is_empty and not self.required: | ||
| return [] | ||
|
|
||
| if is_empty and self.required: | ||
| raise ValidationError(self.error_messages['required']) | ||
|
|
||
| if value and not isinstance(value, (list, tuple)): | ||
| raise ValidationError(self.error_messages['invalid']) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this be simplified to reduce indent levels with something like etc. Simply making sure that the approach is to return a value as quickly as possible by eliminating cases rather than drilling down exhaustively.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes I agree, it's copy-paste from above :( |
||
|
|
||
| for field_value, checkbox_value in value: | ||
| try: | ||
| clean_data.append([self.contained_field.clean(field_value), checkbox_value]) | ||
| except ValidationError as e: | ||
| errors.extend(e.messages) | ||
| # FIXME: copy paste from above | ||
| if self.contained_field.required: | ||
| self.contained_field.required = False | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is worth a comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, at which point do we check that we have all
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also copy-paste :( |
||
| if errors: | ||
| raise ValidationError(errors) | ||
|
|
||
| self.validate(clean_data) | ||
| self.run_validators(clean_data) | ||
| return clean_data | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| $(document).ready(function() { | ||
| var itemTemplate, | ||
| lastItemId, newItemId, | ||
| count = $(".js-widget-item").length, | ||
| fieldNameMatch = $(".js-widget-last-item input:first").attr("id").match(/id_([a-z_]+)_\d+_\d+/), | ||
| fieldName = fieldNameMatch ? fieldNameMatch[1] : null; | ||
|
|
||
| if (!fieldName) { | ||
| throw new Error("Missing field name."); | ||
| } | ||
|
|
||
| lastItemId = fieldName + "_" + (count - 1) + "_"; | ||
| newItemId = fieldName + "_{count}_"; | ||
| itemTemplate = $(".js-widget-last-item").prop("outerHTML"); | ||
| itemTemplate = itemTemplate.replace("id_" + lastItemId + "0", "id_" + newItemId + "0"); | ||
| itemTemplate = itemTemplate.replace("id_" + lastItemId + "1", "id_" + newItemId + "1"); | ||
| itemTemplate = itemTemplate.replace(lastItemId + "0", newItemId + "0"); | ||
| itemTemplate = itemTemplate.replace(lastItemId + "1", newItemId + "1"); | ||
|
|
||
| var $lastItem; | ||
|
|
||
| function addNewItem () { | ||
| var $item = $(itemTemplate.replace(/\{count\}/g, count)); | ||
| count += 1; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Race condition? Should you increase count first, perhaps with Doesn't matter that much, dunno. (actually that's not better, and you'd have to do
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how can it be a race condition? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you click twice really fast to add a new item
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is no + button |
||
| $item.insertAfter($lastItem); | ||
| $lastItem.removeClass("js-widget-last-item"); | ||
| stopListening($lastItem); | ||
| $lastItem = $item; | ||
| startListening($lastItem); | ||
| } | ||
|
|
||
| function startListening ($item) { | ||
| $("input:first", $item).on("change.listWidget", _onLastItemChange); | ||
| } | ||
|
|
||
| function stopListening ($item) { | ||
| $("input:first", $item).off("change.listWidget", "**", _onLastItemChange); | ||
| } | ||
|
|
||
| function _onLastItemChange () { | ||
| if ($(this).val()) { | ||
| addNewItem(); | ||
| } | ||
| } | ||
|
|
||
| $lastItem = $(".js-widget-last-item"); | ||
| startListening($lastItem); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| <ul> | ||
| {% for widget in widgets %} | ||
| {% if forloop.last %} | ||
| <li class="js-widget-item js-widget-last-item">{{ widget|safe }}</li> | ||
| {% else %} | ||
| <li class="js-widget-item">{{ widget|safe }}</li> | ||
| {% endif %} | ||
| {% endfor %} | ||
| </ul> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| <ul> | ||
| {% for widget in widgets %} | ||
| <li>{{ widget|safe }}</li> | ||
| {% endfor %} | ||
| </ul> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docstring/comments explaining what we're trying to achieve and how? As discussed the
enumerateis a very important component and is slightly deviously hidden.