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
Fixing notice when doing batch copy and language change #5317
Conversation
Could you quickly test this at your end? If not fixing it, it should at least improve it... |
same |
I changed this further. If this does not work, I'm really stumped... 😉 |
@Hackwar I tried to reproduce your issue, but I can't... Where do you see the error message? Here I'm using PHP 5.6.3 with |
@Hackwar forget it! I found it... sorry 😊 |
... but, with your PR applied, I'm still getting this: |
…atch-18 Conflicts: libraries/legacy/model/admin.php
Ok, so I looked into this further and we basically have this bug everywhere where we implemented the batch feature... Please test this now again. I guess I caught all. |
@Hackwar Seems to be alright now, Hannes! While you're into the batch thing, can you look at #5304 "Problem 3"? It is a minor thing and in a way a documented "feature", but IMHO the behavior is at least... "not elegant". In #5304 (comment) I explain what I would expect as a normal behavior. Thanks! |
Hi @smanzi, I saw those issues and while we can fix those at some point in time, I would rather not complicate this PR by adding even more to it. Lets first treat this one and then tackle the rest later. |
@Hackwar I totally agree! I was not asking to integrate a fix for that "glitch" into this... It was just an "Hey, take a look at that!" 😄 |
I don't see any regression, so... @test success! |
Tested successfully! Thank you @Hackwar for the fix. 2 tests -> RTC |
Not RTC yet. The PR works for the components patched |
Done. I plan on investing some time into the batch code... There seem to be some more improvements possible, but I will work on that in another PR. |
@test success |
Also tested the other components properly! All occurrences are now considered. @Hackwar Thank you for the fast reaction. Now is should be okay to set to RTC! :-) |
Merging, thanks. @Hackwar |
Fixing notice when doing batch copy and language change
Fixes #5312
This PR updates the $contexts array when items are copied and at the same time, the language of the new copy is changed. This assumes that the context of each item is the same... I'm not sure that is always the case and thus someone else has to review this.
Thanks for the bug report @infograf768
Could you provide test instructions?