- πΊπΈUnited States Greg Boggs Portland Oregon
Given that the module has almost no useful tests, I think it's reasonable to commit such a key feature without having to first build out a test suite for the module. However, even so, the patch does not apply and still needs a reroll.
- Status changed to Needs review
about 1 year ago 7:47pm 7 September 2023 - last update
about 1 year ago Composer require failure - π§πͺBelgium matthiasm11
The following things have been improved in the patch (which applies agains 2.x-dev).
- Add support for all content entities. (patch #4 only added support for nodes and taxonomy terms)
- Fix issue where the base table is used instead of the data_table (f.e. node instead of node_field_data or taxonomy_term_data instead of taxonomy_term_field_data) and keep backwards compatibility.
- Fix issue when using this filter on an entity added through a views relation.
- Add support for multiple filters in the same view, on the base table or a views relation(s).
- Add support for current content language, while keeping backwards support for current interface language.
- Add support for entities having custom entity_keys for the "published", "langcode", "default_langcode" field.
This patch also contains the fixes for the following issues, to prevent merge conflicts or git apply conflicts.
- https://www.drupal.org/project/select_translation/issues/3383945 π PHP 8.2 compatibility RTBC
- https://www.drupal.org/project/select_translation/issues/2867316 π Support content language in the selection mode Needs review
- π΅πΉPortugal joao.ramos.costa
joao.ramos.costa β made their first commit to this issueβs fork.
- π΅πΉPortugal joao.ramos.costa
Hi @matthiasm11,
worked smoothly for 2.x-dev,
thank you!Here is a 'reroll' from #9 for 2.0.0-alpha4.
- π«π·France duaelfr Montpellier, France
I can confirm patch from #12 is working well. Thanks!
It's a bit strange to have merged this issue with π Support content language in the selection mode Needs review but as I needed both I can't complain too much ;)
The maintainer might want you to split your patches to make it easier to review, though. - π΅πΉPortugal joao.ramos.costa
Hi @duaelfr,
Sure. I've opened MR10 to split the patches to make it easier to review.
Here's a path from the diff to one who might want to use it.
Regards,
J.
- π«π·France duaelfr Montpellier, France
Hi @joao.ramos.costa
Thanks a lot! This looks really good! (and also thanks to @matthiasm11 for the initial work!)
I did a review on the code and found a potential issue along with two nitpicks. I hope you don't mind :)On another topic, while splitting the code, did you keep the part about content language? That would fit perfectly in a MR on π Support content language in the selection mode Needs review . Both might be needed for some people ;)
- π΅πΉPortugal joao.ramos.costa
Hi @dualfr,
Thanks a lot for reviewing it. I'll have a look at your thoughts on the review soon, but they seem absolutely crucial at first glance. I can't promise exactly when, but it will be soon.
Regarding the second topic, I think it should definitely be done. It was possible to extract it, but it didnβt apply directly since it fit with the conditionals added in this MR. However, I promise to take a look and open an MR as well.
Thank you !
- π΅πΉPortugal joao.ramos.costa
Hi @dualfr,
mr is updated according to your concerns, thanks! On the other hand, I think an addition fixes the buildOptionsForm, as I couldn't see the other fields using Drupal 10.3.6. I hope no one minds being added here; otherwise, it would have been difficult to test the functionality properly.