Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I have a nested field collection that looks like this:
Category (field collection)
> FAQ (field collection)
> Question (text field)
> Answer (text field)
I can add one or more categories and under each category I can add one or more FAQ items. But, when removing one of those FAQ item field collections it removes all FAQ items under the parent category.
Comment | File | Size | Author |
---|---|---|---|
#22 | field_collection-remove-items-2613584-22.patch | 5.7 KB | juampynr |
| |||
#22 | interdiff.txt | 684 bytes | juampynr |
#13 | field_collection-remove-items-2613584-13.patch | 5.69 KB | juampynr |
| |||
#11 | field_collection-remove-items-2613584-11.patch | 5.6 KB | juampynr |
| |||
#10 | field_collection-remove-items-2613584-10.patch | 5.6 KB | juampynr |
|
Comments
Comment #2
tim.plunkettData loss is critical.
Comment #3
balsamaTechnically, there does seem to be a workaround for this that avoids data loss. If you click the "Add another" button on the parent field collection after clicking delete, the previously erroneously deleted child field collections reappear (without the one that was intentionally deleted). I've verified that clicking save at that point does not result in data loss - and does, as expected, delete the single child field collection.
Maybe this can be a clue when troubleshooting?
Comment #4
juampynr CreditAttribution: juampynr at Lullabot commentedThis is where I am at. I could verify that for nested field collections, the second level elements are removed as expected.
I still need to work in the following interactions for this patch to be complete:
* Adding nested field collections (the wrapper id changed).
* Removing parent field collections.
* Adding test coverage.
Comment #5
juampynr CreditAttribution: juampynr at Lullabot commentedBy the way, my patch is heavily influenced by FileWidget::submit(), which accomplishes something pretty similar when you click at the Remove button of an uploaded file.
Comment #6
tim.plunkettCan we reuse this variable instead of redoing the array_slice?
Every time we do an array_slice it'd be nice to have a comment explaining the 3rd param. This second call says "Go one level up" but has a -2
Otherwise, looks good so far!
Comment #7
juampynr CreditAttribution: juampynr at Lullabot commentedI made these array indexes as meaningful as possible, plus added comments. Now I will continue with the remaining tasks listed above.
Comment #8
juampynr CreditAttribution: juampynr at Lullabot commentedAdjusted wrapper identifiers so now both the Remove in nested field collections and the "Add another item" button in parent ones work. I am working now in fixing the Remove button of a parent field collection.
Here is a screenshot with annotations describing what I am up to:
Comment #9
juampynr CreditAttribution: juampynr at Lullabot commentedThis patch simplifies the logic to generate wrapper identifiers. I am still debugging why the Remove button works sporadically. At least it does not delete unexpected field collections.
Comment #10
juampynr CreditAttribution: juampynr at Lullabot commentedThis patch works much better but when you play with the add and remove buttons for a while, sometimes you see stalled values that were removed (not saved in the database, but temporarily in $form_state). I would like to get some feedback as it works much better than what we currently have in the module.
In the meantime, I will try a different approach which is to re-paint the whole field collection when any add/remove button is clicked. This should maintain values consistent.
Comment #11
juampynr CreditAttribution: juampynr at Lullabot commentedOops, fixed a couple typos in comments.
Comment #12
juampynr CreditAttribution: juampynr at Lullabot commentedWe already have code in FieldCollectionEmbedWidget::removeSubmit that does what I am doing at FieldCollectionEmbedWidget::ajaxRemove. I am debugging the former to see if this is the reason of some weird behaviors that I have found when you add and remove several items within the same page load.
Comment #13
juampynr CreditAttribution: juampynr at Lullabot commentedHere it is. I am not attaching an interdiff because this patch follows a different approach by doing the following:
* Fixes AJAX wrappers so they are unique.
* Fixes indexes in the
removeSubmit()
handler.* Simplifies
ajaxRemove()
considerably, making it just to return the field collection item.Comment #14
yannickooThank you for you patch Juampy! It works well but when I'm trying to add new field collection items and delete them I experienced something funny with creating new items:
Comment #15
juampynr CreditAttribution: juampynr at Lullabot commentedThanks @yannickoo! I will debug that scenario and report back.
Comment #16
juampynr CreditAttribution: juampynr at Lullabot commentedField collections are piling up within the widget state because the Remove submit handler is not removing them as expected. I am looking at the Drupal 7 code to understand how this should work.
Comment #17
juampynr CreditAttribution: juampynr at Lullabot commentedDrupal 7 piles up field collections in widget state but this does not seem to affect how the form gets rebuilt.
Here is how to reproduce the error in drupal 8:
Given a node multiple field collection item with a text field:
1. Go to add content and click on create a node.
2. Fill out the first field with a.
3. Click on Add another item.
4. Enter a b.
5. Remove the item with a.
6. Remove the item with b.
7. Enter c, click on Add another item.
- Expected: a new empty item is added.
- Actual: a new item containing c is added.
It looks like there is something when the form rebuilds that causes this. I am debugging the code.
Comment #18
juampynr CreditAttribution: juampynr at Lullabot commented@yannickoo, after debugging, I think that the issue that you describe at #14 is a different bug than the one described at the top of the issue. I could reproduce your issue without applying my patch at #13. Therefore, let's go step by step and set this to needs review again. I will create a new issue with your feedback.
@tim.plunkett, can you review this patch please?
Comment #19
juampynr CreditAttribution: juampynr at Lullabot commentedFollow up https://www.drupal.org/node/2636198
Comment #20
tim.plunkettNice! That explains a lot of the weirdness I was seeing while debugging.
This should be
@see self::removeSubmit()
, and should have a newline above it. I can fix on commit.I think this looks good. Going to check with some coworkers about manual testing before I commit it.
I agree that the "add another item" bug can be a follow-up.
Comment #21
phenaproximaTested this manually and I can confirm that it fixes the issue described in the IS. Nice!
Comment #22
juampynr CreditAttribution: juampynr at Lullabot commentedHere is another patch with @tim.plunkett's suggestion.
Comment #23
balsamaI've also manually tested this and confirmed this works and it's integrated into our D8 Contrib build.
Marking as RTBC.
Comment #24
tim.plunkettCommitted, thanks!
Comment #26
yannickoo<3