-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cannot save content elements with max items set to 1 #36
Comments
Hi @andreashager, Thanks for your report. Unfortunately I'm unable to verify the problem. Actually there is even an existing test for that scenario. Would you mind to share your backend layout configuration and your TYPO3 and content_defender versions? |
Thanks for your reply - sure I can do that 😄 Here are my versions:
and here is my backend layout configuration:
|
I also did an update now to |
There wasn't any technical update just compatibility for TYPO3 9.1 |
Ah oke I see 😅 😀 |
@andreashager With the provided config I'm able to create content records in each column without problems. Have you installed other extensions (gridelements?) as well? What is the exact error your are seeing? What are you exactly doing? |
Do you have hidden records in the column(s) already? Do you see "Show hidden content elements" at the bottom of the page view? Maybe you can check the checkbox to see hidden elements. |
I had now a look in the database if there are already deleted/hidden entries on this page but there are only 2 entries in tt_content for this pid each with different colPos. So this should be correct 😄
Extensions I installed (I already deactivated all of them to check if the problem still exists, yes it does): What I am exactly doing?
|
Hi @andreashager, I'm still not able to reproduce the problem. Is there any way I can test it in your system? |
@IchHabRecht
In this line the problem occurs: |
I've got this problem, too. I'm using:
When saving the record, the colPosCount gets increased by:
The method returns true , so the if evaluates to false (which means the record is allowed here).
But this method call increases the internal counter: content_defender/Classes/Hooks/AbstractDataHandlerHook.php Lines 71 to 78 in 85372af
Which means that the later condition here evaluates to true, which means the exception is thrown because content_defender/Classes/Form/FormDataProvider/TcaColPosItems.php Lines 78 to 80 in 85372af
The funny thing is, that the content element was already saved like it should! Maybe the last condition should check the I would create a PR, but I'm not that experienced in the TYPO3 internals, so I'm not sure what the right thing to do here would be. But if I change the last if condition to this, the error is gone: if ($result['command'] === 'new' // or: !== 'edit'
&& !empty($columnConfiguration['maxitems'])
&& $columnConfiguration['maxitems'] <= $this->contentRepository->countColPosByRecord($record)
) { |
I just gave it a try with There's no error when I create two elements. But: When I try to edit any one of the two existing elements, the Exception is thrown with:
which is clearly wrong. When I apply my patch (adding |
Hi @jdreesen, Are you using any grid elements on the mentioned page? What kind of content type have you created? |
@andreashager Are you using gridelements as well? |
The page I was testing with has a custom backend layout and the The content element I used in the test (which is the only allowed type CType here) is a custom tt_content element which has only a |
Hi @jdreesen, Would you mind to share your backend layout configuration as well as your content definition? If you want, you can contact me on slack. |
I just created a PR with the code that fixes the problem for me. Also, I've sent my backend layout and TCA to @IchHabRecht via slack so she's hopefully able to recreate the problem an verify the fix. |
@IchHabRecht No I don't use grid elements on the complete website and the extension is not installed! 😃 |
I'm able to reproduce the issue now and working on a fix. |
Sounds great, thx :) |
Would you mind to test branch https://github.com/IchHabRecht/content_defender/tree/fix-maxitems-handling and see if everything works out for you and your configuration. Thanks in advance! |
I just tested it with the branch and it works for me now. Great work, thx! :) |
@IchHabRecht Same for me, I tested it with the branch you provided, works great :) Thanks! Can I ask what the problem was? :) |
sure :-) Content Defender is called before the record is saved to decide whether an element can be saved or not. Due to the early access the element has no uid yet, just some id starting with NEW. After saving the new record to the database there is no redirect that would enforce a new script start, but the EditController saves and opens the record in one request. So the DataProvider used in FormEngine context was told, there is already one element (the new one) in the column but didn't found out that is the same record due to the missing (new) uid. That's why the exception was thrown then because the column is loaded and cannot store any new record. The solution is to save the id of the new record inside content_defender and replace it after DataHandler work with the real uid of the record. Then the FormEngine part can find out that the record that should be shown is the one in the column. |
Ah oke I undertand :) Thanks for your help, now everything works! 😀 |
Hallo,
there is a problem when I set the maxitems to 1 in the backend layout. Then the only content element for this colPos cannot be saved anymore and the following message is shown:
The problem only occurs if the maxitems is set to 1, when it is set to 2 I can add and save both content elements without any error.
If you need further information pls feel free to ask me 😀
The text was updated successfully, but these errors were encountered: