Account created on 19 May 2010, about 14 years ago
#

Merge Requests

More

Recent comments

πŸ‡¬πŸ‡·Greece vensires

Thanks for the validation @alfthecat! I'm setting it as RTBC.

πŸ‡¬πŸ‡·Greece vensires

Since we have so many validations that #26 works, I set it as RTBC.

πŸ‡¬πŸ‡·Greece vensires

I copy the text from the README.md file of the module:

### Usage with Drupal 10.2

For Drupal 10.2, the drupal/remote_stream_wrapper (issue 3437974) patch needs to be removed.
If you require this module in Drupal 10.2 you must unset the patch in your composer.json.\
You can do this with the following command:
```bash
composer config --merge --json "extra.patches-ignore.drupal/media_avportal" '{"drupal/remote_stream_wrapper": {"Drupal 10.3.x only - see media_avportal/README.md for 10.2.x - https://www.drupal.org/project/remote_stream_wrapper/issues/3437974": "https://www.drupal.org/files/issues/2024-06-21/drupal_10_3_deliver_signature_change-3437974-2_0_0-18.patch"}}'
```

Feel free to reopen the ticket if I didn't correctly understand your issue.

πŸ‡¬πŸ‡·Greece vensires

I just opened πŸ’¬ Offering to co-maintain Remote Stream Wrapper Active in case anyone is interested...
(@apotek, I mentioned you too)

Thank you @cmlara for your hint!

πŸ‡¬πŸ‡·Greece vensires

Count me in @apotek. I don’t know how much help I will be able to offer but I will definitely try.

πŸ‡¬πŸ‡·Greece vensires

@platinum1 could you also check it and if it's ok, then set it as RTBC?

πŸ‡¬πŸ‡·Greece vensires

Adding a patch targeting 2.0.0. Not to be used but for anyone required to use only stable versions.

πŸ‡¬πŸ‡·Greece vensires

I hid the MR to clarify that the patch is the one working. RTBC from me too.

πŸ‡¬πŸ‡·Greece vensires

vensires β†’ changed the visibility of the branch 3366135-nulllogger-not-implements to hidden.

πŸ‡¬πŸ‡·Greece vensires

This is actually a long standing issue and other than turning in into a MR so that we can easily update it with the latest module commits, I don't think this will ever get into core. The main reason is that GROUP_CONCAT() is only valid for MySQL databases. PostreSQL has STRING_AGG() and SQL Server requires some strange stuff to accomplish it.

So, from what I understand, we have only a few options here.

Either implement a solution taking into account the database type, either invoke a hook/event/plugin/something to allow other custom or contrib modules to add their own solutions. Contrib modules are more easy to target a specific database than core.

πŸ‡¬πŸ‡·Greece vensires

Since this is a really big issue when first trying to install the module and the patch from #26 actually solves it I set it as "Needs review".

As for the considerations mentioned in #32, I think that it there is something more to address, it should be done in a follow-up issue.
But what do the rest of you believe?

πŸ‡¬πŸ‡·Greece vensires

Both changes described above are addressed in the MR.
Drupal's guidelines also suggest updating *.api.php file but since we don't have one, it's ok as it is.

πŸ‡¬πŸ‡·Greece vensires

vensires β†’ made their first commit to this issue’s fork.

πŸ‡¬πŸ‡·Greece vensires

@heddn, the patches, the MR and the whole issue in general targets the 2.x version.
I agree that it's not clear if and what is RTBC; I'd rather set it as "Needs review". From what I understand and remember, MR!55 and patch in #23 πŸ› Ajax facet adds q and _wrapper_format to the browser URL Needs review are good candidates for committing them.

πŸ‡¬πŸ‡·Greece vensires

This is also useful to have in views queries when the base table is the one of Paragraphs.

πŸ‡¬πŸ‡·Greece vensires

Thanks for your feedback @redwan-jamous. You are right!
I am setting this as RTBC.

πŸ‡¬πŸ‡·Greece vensires

Closing this as working as designed since it's actually Entity Reference Revisions module's issue.
You did well to add it as an issue here though! Thanks!

πŸ‡¬πŸ‡·Greece vensires

Patch in #5 works great. I'm setting this as RTBC!

Extra things done:
- Updated the MR to match its changes
- Changed the info.yml file since according to the documentation β†’ the core/once is not available before 9.2.0; this means the new version won't support Drupal 8.8.

Also attaching patch from latest MR diff.

πŸ‡¬πŸ‡·Greece vensires

vensires β†’ made their first commit to this issue’s fork.

πŸ‡¬πŸ‡·Greece vensires

I opened the two MRs in order to better understand the changes proposed in each one.
It seems that MR!13 is actually in WIP state.
MR!12 on the other hand seems to be the one we should go on with.

(@chrisachrist Don't hesitate to correct me if I'm wrong though)

I still have to validate that MR!12 is really working..

πŸ‡¬πŸ‡·Greece vensires

When we first stepped on this issue, we couldn't find out why this is needed since the functionality exists in core. I edit the summary to clarify things for other people too...

πŸ‡¬πŸ‡·Greece vensires

Corrected the description of the OpenModalDialogCommand class.

πŸ‡¬πŸ‡·Greece vensires

I personally think that this last comment is not related to the issue tackled here unfortunately.

Your issue seems to be that the there are still paragraph entities having as parent ID your host entity which - from a query perspective - is correct. Maybe your solutions lies on the functionality of paragraphs to remove orphaned paragraphs.

πŸ‡¬πŸ‡·Greece vensires

I have created a new MR with the changes from #3431221-3: allow form elements to override Chosen library options β†’ but for 4.0.x-dev version.
No further changes exist in my code.

PS: It seems that the tests are failing but I am not sure if they break due to this code or if they were already broken from the past; in the later case we should handle them in a different issue.

πŸ‡¬πŸ‡·Greece vensires

@thanos-ntelis what's the status of this MR? I still see some TODO comments in the code.

πŸ‡¬πŸ‡·Greece vensires

vensires β†’ changed the visibility of the branch 3444143-change-webservice-urls to active.

πŸ‡¬πŸ‡·Greece vensires

vensires β†’ changed the visibility of the branch 3444143-change-webservice-urls to hidden.

πŸ‡¬πŸ‡·Greece vensires

Is this still valid or is this outdated?

πŸ‡¬πŸ‡·Greece vensires

Keeping it as RTBC but, are we sure a composer.lock file should exist?

πŸ‡¬πŸ‡·Greece vensires

From what I checked it seems the changes should break anything in older versions.

πŸ‡¬πŸ‡·Greece vensires

vensires β†’ made their first commit to this issue’s fork.

πŸ‡¬πŸ‡·Greece vensires

Thanks you for your time on this. Setting as RTBC then, based on your feedback.

πŸ‡¬πŸ‡·Greece vensires

I updated the MR13 to better understand the changes from the patch. Needs further testing and review if we are going to use it instead of the patch; just to be sure I did the rebase correctly.

As for the patch provided in #15 πŸ› Take the selected filters into account, support multiselect filters and make 'delete all' text translatable Needs review only the {{ options.reset_link.title|t }} troubles me. Can't we avoid it using configuration translation?

πŸ‡¬πŸ‡·Greece vensires

So we can hide the other MRs and only keep the MR162 as relevant and working though it's currently marked as draft?

πŸ‡¬πŸ‡·Greece vensires

I totally agree that we shouldn't go on with any changes in the code. Just document. And your MR succeeds in this.
So, I set it as RTBC ;)

πŸ‡¬πŸ‡·Greece vensires

It's a really small but working change, so if no one else has any objection, I set it as RTBC.

πŸ‡¬πŸ‡·Greece vensires

I personally think that if we moved this functionality away, it will be like we deprecate it since it's been here for years and developers expect it to exist by default with the Field Group module.

Unless you do want to deprecate it; that's another discussion...

πŸ‡¬πŸ‡·Greece vensires

Works fine for me! Setting as RTBC!

πŸ‡¬πŸ‡·Greece vensires

Added as default layout plugin "layout_twocol" which is also the one used in tests in core Layout Builder module

πŸ‡¬πŸ‡·Greece vensires

I have created all the different issues described as children-issues of this meta.

Production build 0.69.0 2024