🇩🇪Germany @marcoliver

Oops, my bad! MR 8277 now targets 11.x

marcoliver → changed the visibility of the branch 11.x to hidden.

Sure thing! I created a new MR (8257) targeting 11.0.x.

Thanks @Amandeep123. Merged. Will tag a new version shortly.

Hi @mattea.turnbull! I created an issue fork and a merge request. The change seems pretty straight forward, but could I still bother you to take a look, whether your need is actually addressed?

If you're happy with the change and do not encounter any issues, I'll be happy to tag a new release shortly!

Sounds like a good enhancement. I'll take a look at the next opportunity!

Also gave the latest commit another look and I share divya.sejekan's confusion.

Ideally, shouldn't the entries in the menu be in the same order the entities would be on the entity list page? A.k.a. using the NodeType sort() function?

See MR.

Basically just Ctrl-Shift-F'ed Core for drupalGet\(.*,.*,.* and then manually combed through the results to find any offending calls.

marcoliver → created an issue.

Looks good. RTBC.

Looks good to me, even though I'm now seeing a depressingly large number of errors, but I guess this is something that could/should be addressed in something like https://www.drupal.org/project/tome/issues/3406561 ✨ Improve error messages to be clearer about the path causing the problem Active

Marking as RTBC.

Merged-in the latest 8.x-1.x commits.

MR looks good to me, since 8.x-1.12 dropped support for D8, so the core key can be dropped. Marking as RTBC.

marcoliver → made their first commit to this issue’s fork.

Looks correct now. Marking RTBC.

Fixed one failing functional test by adding another permission so sub-menu-items would show up.

Not sure how we should deal with the remaining issue. AdminToolbarAdminMenuTest extends ToolbarAdminMenuTest. ToolbarAdminMenuTest (and a bunch of other tests in Core) use drupalGet() and set HTTP headers by sending them as a complete string ("header-name: header-value"), for example here. But drupalGet() expects the headers to be sent with the header name as the key and the value as, well, the value.

This should probably be addressed as a Core issue, no?

marcoliver → made their first commit to this issue’s fork.

Reverted back to using the older MASTER_REQUEST constant, which is deprecated but still usable in both D9 and D10.

Will tag 3.1.33 in a moment.

marcoliver → made their first commit to this issue’s fork.

FYC, here's an MR.

"Clear/recommended filters" buttons had no type attribute set, making them submit buttons by default. So they'd implicitly be "clicked" whenever the form was submitted, toggling the default filters.

marcoliver → made their first commit to this issue’s fork.

Rebased MR looks good to me. Current pipeline failures seem to be unrelated and out of scope for this task, so marking as RTBC.

@enchufe Even without your patch I am seeing the sitemap.xml file being created in the static folder.

I partially agree with @samuel.mortenson insofar that the "empty" display of the XML seems to be a server issue, not necessarily Tome-related.

But the module is nevertheless lacking a way to discover and save the XSL stylesheet for the sitemap.

I have created a first attempt at a solution for this in MR 24, kindly asking for review / feedback.

marcoliver → made their first commit to this issue’s fork.

I have created a MR with a fix similar to what @dineshkumarbollu proposed, but also avoiding the warning @jrochate mentioned.

I believe providing an empty fallback value here in case there are no target_bundles defined is fine. $field_target_bundles is used for one specific check only, which we're not going to pass anyway if $field_target_bundles has an empty value. Afterwards it is not used again.

I played with the scenario a bit and the change does fix the error but does not seem to introduce any new issues.

I'd greatly appreciate it if somebody else could give the MR a quick look.

marcoliver → made their first commit to this issue’s fork.

Version 2.2.0: Removed the library-dependency on drupal/js-cookie and required it as a JS build dependency.

Fixed in version 2.2.0.

Fixed in version 2.2.0.

marcoliver → created an issue.

marcoliver → created an issue.

Okay, marking this as fixed. Will tag version 3.0.0 in a moment.

marcoliver → made their first commit to this issue’s fork.

Never mind! I was missing the "Simplenews Newsletter" overrides! After importing and enabling those, I began to see the per-newsletter policy settings!

Closing this.

marcoliver → created an issue.

Moin @avogler! I took some time yesterday and opened a 3.0.x branch to specifically work with Simplenews 4.

https://git.drupalcode.org/project/degov_simplenews/-/tree/3.0.x?ref_typ...

This version of the module is aware of the difference between the signup form block and the manage form page. I also had to make some other minor changes to a few form hooks. In the end that's why I opted for a separate branch for SN4 support.

I've already asked a colleague of mine to give that branch a spin to see if there are any problems I missed or introduced in this version. I'd greatly appreciate it if you could also take a look from your side!
Thanks in advance!

Thanks for testing, and thanks for José Trindade to raising this issue! Merged. Will tag a new release shortly.

Thanks for testing! Merged. Will tag a new release shortly.

marcoliver → created an issue.

@fathima.asmat Good catch, thanks! No particular reason, I just didn't see that there was an update available.

I bumped the versions once more. The JS tests are now run in the pipeline as well.

marcoliver → created an issue.

Ah okay, I can see it now! When I am on the page to manage my subscriptions, it loads the page form, which then explodes. Okay, your change makes sense here!

But I think we'll need to do some more work on this issue then. Because right now I can be an anonymous user with subscriptions and Simplenews' manage feature allows me to view the form, but degov_simplenews has no concept of this pseudo-authenticated user yet. Thus the name fields are empty and the Terms-and-conditions checkbox appears again.

I'll see if I can spend some more time on this next week!

marcoliver → created an issue.

Hi avogler!

Thanks for your patch! I just had a look at it and I'm not sure what motivated this change: https://git.drupalcode.org/project/degov_simplenews/-/merge_requests/2/d...

From what I can tell, the function you removed is still present in the 4.0.0 branch of simplenews.

Can you please let me know your reasoning behind the change?

Updated against 1.0.x, MR is mergeable again.

I updated the MR from #12, incorporated the changes from #8 and worked on the rest of the reported issues.

drupal-check now shows me no errors!

marcoliver → made their first commit to this issue’s fork.

Created an issue fork from the patch. Can confirm that only enabled views show up in the menu now.

Marking RTBC.

marcoliver → made their first commit to this issue’s fork.

Synced 3.x and 3420788-search-links-always in the issue fork.

Tested the behavior and it looks fine (video attached).

Marking RTBC.

marcoliver → made their first commit to this issue’s fork.

Looks good to me! The issue fork solves the problem described in the issue body.

Comments #7 and #8 may be valid, but are out of scope for this task.

Marking RTBC.

Kristen Pol → credited marcoliver → .

Oops, my bad, should have waited for the pipeline!

Removed the offending pager, pipeline's green now!

Okay, I updated the issue fork against the latest 2.0.x-dev and made the following changes to address the pipeline issues:

• changed the format of the deprecation error message in AddressDefaultFormatter.php (pulled the version numbers from thin air, feel free to change them!)
• phpstan-ignored the fallback \Drupal call to assign the ConfigFactory if it is missing from params, as I don't see how this could be solved via DI

MR is up-to-date again

Hi!

I created an issue fork. I took the idea from the patch and slightly expanded upon it. Mainly, instead of just not breaking the page, the widget now displays a message that the content type is not managed by a workflow.

I then added a functional test that replicates the scenario outlined in #6 🐛 Call to a member function getTypePlugin() on null in Drupal\scheduled_publish\Plugin\Field\FieldWidget\ScheduledPublishWidget->formElement() Needs work and checks for part of that explainer text.

I'm currently seeing an issue with the testing step phpunit (next major). It seems to me to be unrelated to the module itself, but maybe someone else has an idea here.

marcoliver → made their first commit to this issue’s fork.

Was able to reproduce the warning & the patch fixes the issue. Marking RTBC.

Looks good! The level 2 parent of the current page is now highlighted as expected. RTBC.

Hi! I updated the Issue Fork against the current state of 2.0.x again (Commit). I also removed a line in the new code that changed the output and caused some unit tests to fail (Commit).

What I'm unsure of though is: we've introduced some changes that cause warnings in phpcs and phpstan (see pipeline). How do we deal with those? Fix? Or just ignore them for now, as we'll remove the deprecated stuff in a later version anyway?

Oh, otherwise the functionality itself still looks good!

With the lack of info in the issue body and @kufliievskyi's work trying to reproduce the problem, I'll close this issue.

I agree with @kufliievskyi. This seems to be solely an issue with the version constrained used to require the module into the project. Closing.

Hey, no problem at all! Thanks for voicing your concerns!

Hey everyone, I updated the issue fork with a change to address the JavaScript issue (using native bind() instead of the jQuery proxy now) and one to remove the filter added by Core to the bottom of the list.

I decided to unset the Core filter in this case, since this module provides its own filters anyway. Feel free to discuss / change this if you think otherwise.

Hi pbouchereau,

thanks for your message! I tried reproducing the scenario you outlined, but wasn't able to create a problem.

When a user's directly assigned term-associations are changed, permissions_by_term_user_form_submit updates these associations and rebuilds node access.

When I change, which roles/users have permission to access a term, permissions_by_term_submit is triggered and also rebuilds node access if necessary.

Can you please elaborate on where you see the potential problem?

Looks good, thanks!

marcoliver → made their first commit to this issue’s fork.

Patch worked fine for me as well. Created an issue fork. Marking as RTBC.

marcoliver → made their first commit to this issue’s fork.

Hi everybody! I went ahead and created an issue fork in Gitlab for this issue.

The patch still applied fine, plus I made some minor changes re: coding standards.

Would be cool to see this feature as part of a future release!

marcoliver → made their first commit to this issue’s fork.

Hi Rajab Natshah!

Thanks for creating this issue and your patch!

I have created an issue fork based on your patch (and some code-cleanup). Everything seemed fine from what I can tell. But can you please give the issue fork a quick spin to see, if the fix still works for your particular problem?

marcoliver → made their first commit to this issue’s fork.

Hi everyone, I went ahead and merged the fork with the new setting. It will be released in 3.1.31.

Marking this as fixed. Is this issue persists for anyone on 3.1.31, feel free to reopen it or open a new one.

Closing as outdated, since we're way past Drupal 9 at this point.

I spent some time reproducing the problem. I am able to reproduce it, but just changing the selector in the JavaScript as you described does not seem to do the trick, since autocomplete_deluxe appears to store the data in a different structure of form elements.

Can you please provide not only the changes that work for you in the patched bundle.js, but also in the source file (dom-client.prototype.js I'd assume)? That would make it significantly easier to find out how we can make this a universally applicable solution for all, with and without autocomplete_deluxe installed.

Hi mohammedOdeh, can you please provide a bit more info about the specific versions of Drupal Core and symfony/http-kernel you are using? Because from what I can see, there should be default parameters already in place and no need for us to provide an empty string here.

Will be released with 3.1.31

Thanks for your work! I made some adjustments to the issue fork (closing brace was missing & I changed the !is_null() to an is_countable() as that seems more precise to me).

Merged.

marcoliver → made their first commit to this issue’s fork.

Hi! The problem described in this issue was addressed in https://www.drupal.org/project/permissions_by_term/issues/3354478 🐛 Regression updating to 3.1.22 for Drupal 9.x using loadTemplate from twig Fixed

Updated the failing tests to work with the new VariationCacheAPI.

marcoliver → created an issue.

Issue fork is merged. Will be released in 3.1.31.