🇺🇸United States @papagrande

US West Coast
Account created on 21 July 2011, over 13 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States papagrande US West Coast

#3445820 📌 \Drupal calls should be avoided in classes Active should be merged in first as there is a lot of overlap in the MRs.

🇺🇸United States papagrande US West Coast

Conflicts fixed. Please review again.

🇺🇸United States papagrande US West Coast

Blocked by https://www.drupal.org/project/eloqua_api_redux/issues/3454756 📌 Drupal 11 compatibility fixes for eloqua_api_redux Needs review

🇺🇸United States papagrande US West Coast

I just merged in another MR. Can someone address the merge conflicts and any new PHPStan errors?

Thanks.

🇺🇸United States papagrande US West Coast

I also had problems with installation and now get out of memory errors when running composer up in both ddev and on my MacOS host.
PHP Fatal error: Allowed memory size of 1610612736 bytes exhausted (tried to allocate 20480 bytes) in /var/www/html/vendor/php-tuf/php-tuf/src/CanonicalJsonTrait.php on line 46

FWIW, here is my data on one client's installation after several attempts:
Packages: 118
TUF metadata: 5.3M

🇺🇸United States papagrande US West Coast

Could we add the release date to the view on e.g. https://www.drupal.org/project/drupal/releases ? It saves a click into an individual release that does show the release dates.

🇺🇸United States papagrande US West Coast

Looks like 4de095cc31b8101e286bfa86d03b97e04ee44f23 ( https://www.drupal.org/project/page_manager/issues/3362561 🐛 Path has unnecessary query appended. Fixed ) broke it somehow.

🇺🇸United States papagrande US West Coast

After seven years of no comments, I think we can close this out.

🇺🇸United States papagrande US West Coast

@alex.skrypnyk, thanks for your testing. Since #27 when I had v2 installed, I've updated to v3 and retested. Now all the lines with trans pass. I can remove the folder exclusions that I had as a temporary workaround.

🇺🇸United States papagrande US West Coast

The patch in #8 no longer applies to the latest dev. Needs reroll.

🇺🇸United States papagrande US West Coast

+1 for this.

I've started using this on a client site and had to customize it a bit for Drupal:

For spacing:

$ruleset->overrideRule(new TwigCsFixer\Rules\Whitespace\IndentRule(
    spaceRatio : 2,
    useTab : FALSE,
));


And for now I've had to exclude templates that use {% trans %} because it isn't defined in the default rules, and I couldn't get the ignore next line comments to work.

🇺🇸United States papagrande US West Coast

@andresgmh, thanks for the new patch, but a merge request will be easier to review and collaborate on. At a minimum, please provide an interdiff, https://www.drupal.org/docs/develop/git/using-git-to-contribute-to-drupa... , for your new patch. Thanks.

🇺🇸United States papagrande US West Coast

Removed extra word.

🇺🇸United States papagrande US West Coast

Ready for review and testing. For now, I'm ignoring the phpstan warning that will be fixed in 3445820 📌 \Drupal calls should be avoided in classes Active .

🇺🇸United States papagrande US West Coast

PapaGrande made their first commit to this issue’s fork.

🇺🇸United States papagrande US West Coast

Moving the example endpoint to a sub-module is a good idea. Because I don't use this module anymore, I welcome a merge request to make this happen.

🇺🇸United States papagrande US West Coast

For anyone who stumbles into this issue from searching, check to make sure that the 'view own unpublished content' permission is set for the role.

🇺🇸United States papagrande US West Coast

I'm getting sporadic reports from a customer of this error on a hosted site with a webform using a file upload field, but can't reproduce the error myself, either locally or in production. Can others provide information about:

  • Do the errors happen in all browsers?
  • Are you logged in or not? Do the errors happen for anonymous users?
  • What AJAX settings are enabled in the webform?
  • Is the site local or hosted? If hosted, which provider?
  • Is JavaScript aggregation enabled?
  • Is a CDN in front of the site? If so, which one and are there special caching rules in place?
🇺🇸United States papagrande US West Coast

This module could really use automated functional tests, but I manually tested and reviewed the code. Thanks, @aaron.ferris.

🇺🇸United States papagrande US West Coast

Based on https://www.drupal.org/project/webform/issues/3112506 , it looks like we'd have to build this out ourselves.

🇺🇸United States papagrande US West Coast

After some discussion, we figured out we can use the getForm() method on the eloquaFormsService using the form ID stored in the handler to see if the API is reachable. We can use that to display the API connection status in the handler summary along with a link to the settings page if it is not.

🇺🇸United States papagrande US West Coast

Based on https://www.drupal.org/project/eloqua_api_redux/issues/3358253#comment-15571333 📌 Provide a message to the end user when the authentication token fails Needs review , let's add a refresh token check to the summary as well.

🇺🇸United States papagrande US West Coast

I'm calling this ready on the API side.

On the webform_eloqua handler we can add better messaging in #3440055 Show Eloqua Form Name in Webform Handler Summary Needs review

🇺🇸United States papagrande US West Coast

MR #5 conflicts with #4 in #3358253 📌 Provide a message to the end user when the authentication token fails Needs review , which is higher priority so I'm postponing until that MR is merged in.

🇺🇸United States papagrande US West Coast

@CostinVieru, is this still a problem? Can you provide any more steps to reproduce this on a generic form?

🇺🇸United States papagrande US West Coast

Should this and https://www.drupal.org/project/entity_language_fallback/issues/3407832 🐛 src/FallbackController.php error on PHP8.2, Deprecated function: Creation of dynamic property RTBC be merged together and one of them marked as duplicate? That would be easier for the maintainers to review and commit.

🇺🇸United States papagrande US West Coast

Should this and https://www.drupal.org/project/entity_language_fallback/issues/3421360 🐛 PHP 8.2 Deprecation errors for dynamic properties RTBC be merged together and one of them marked as duplicate? That would be easier for the maintainers to review and commit.

🇺🇸United States papagrande US West Coast

The link to \Drupal\Core\State\State is broken, but I'm not sure where it should go now.

🇺🇸United States papagrande US West Coast

@tony.gutierrez.bf, would the merge request in https://www.drupal.org/project/webform_eloqua/issues/3438008 Provide a message to the end user when the authentication token fails Needs review solve this issue for you by providing feedback about the Eloqua API tokens?

🇺🇸United States papagrande US West Coast

For anyone else having trouble updating to 1.1.6 and beyond, remove any special settings in repositories or conflict in your composer.json file that were added to make the patch work.

🇺🇸United States papagrande US West Coast

Unfortunately, this is not fixed.

- media_library_theme_reset:media_library_theme_reset needs to be removed from bootstrap_styles.info.yml to get rid of the dependency.

🇺🇸United States papagrande US West Coast

The tip in #15 is helpful (as is https://www.drupal.org/project/drupal/issues/2358537#comment-12648753 Do not require a 'title' field Needs work ), but this issue does duplicate several other currently open issues.

🇺🇸United States papagrande US West Coast

I ran into this issue on a client site that was spewing "User error: "update" is an invalid render array key in Drupal\Core\Render\Element::children() (line 98 of /var/www/html/web/core/lib/Drupal/Core/Render/Element.php)" PHP errors because of conditional visibility settings on a computed twig element. Implementing the workaround in #3 removed the errors.

🇺🇸United States papagrande US West Coast

Ah, thanks. I forgot about updating to the dev version.

The patch applies and the error goes away. Beyond that, I don't know this module well enough to properly review the changes.

🇺🇸United States papagrande US West Coast

I tried to apply the patch from MR!4 and it failed. I'm very confused because the patch I downloaded from Gitlab doesn't look the same as https://git.drupalcode.org/project/msqrole/-/merge_requests/4/diffs#note....

🇺🇸United States papagrande US West Coast

Thanks for committing the fix. Can we get a new release?

🇺🇸United States papagrande US West Coast

@andresgmh, thanks. This looks like a useful feature.

A few requests:

  1. If you are still updating your code, please assign yourself this task.
  2. Since Drupal is moving to Merge Requests on Gitlab, would you mind opening one instead of using patches? It's not a requirement, but it makes reviewing much easier.
  3. This module needs test coverage. Ideally, we would have tests to make sure this new feature doesn't cause regressions, but at a minimum we should test to confirm this feature is working.
🇺🇸United States papagrande US West Coast

Looks great! Thanks @Lendude and @HeikkiY.

🇺🇸United States papagrande US West Coast

For others' reference, a client had a link field that was triggering this error on six nodes. The status page had no errors about the field and the configuration status was clean.

Resaving the nodes didn't fix the problem. Nor did copying data via SQL from the field table to the revision table. Here's what did:

  1. Resave the field settings.
  2. Resave the field storage settings.
  3. Each node lost the link field data so copy and paste the link data from another server.

And after resaving the field, the configuration status still had no changes between database and file.

🇺🇸United States papagrande US West Coast

Same problem for me--updated to D10 and RSS feeds break. And the workaround in #5 fixes it.

Based on git blame it looks like the change was introduced in https://www.drupal.org/project/drupal/issues/2921810 🐛 Allow TimestampFormatter to show as a fully cacheable time difference with JS Fixed . Perhaps the Views RSS row formatter needs tweaking to exclude the <time> element.

🇺🇸United States papagrande US West Coast

Linking to the core issue (#2546212) that if it ever gets committed, is the "correct" way to make configuration items translatable.

@opdavies, how can I help get this committed to 2.0.x? It's a blocker for me to upgrade.

🇺🇸United States papagrande US West Coast

@keithdoyle9, this ended up being a problem with too many cache tags on that client's page. Oddly, Cloud Next was worse than Cloud Classic when using the acquia_purge module.

I solved it by removing the individual cache tags with the getCacheTagsToInvalidate() method on the custom entity. The entities are populated and displayed as a complete group so individual entity tags aren't needed.

However, the problem remains in the module in that it shouldn't have a fatal error.

🇺🇸United States papagrande US West Coast

Is this still a problem in Drupal 10? With no comment for four years, it would seem folks are working with the current settings.

If not, please add steps to reproduce including operating system and versions.

Bumping the priority down because of the lack of comment or progress.

🇺🇸United States papagrande US West Coast

Is this still a problem in D10 or even D7? If not, then we should close it or reassign to D7.

🇺🇸United States papagrande US West Coast

After reading the relevant code, "items" makes sense in context and !4956 looks good.

Thanks, @sourabhjain.

🇺🇸United States papagrande US West Coast

From @andypost at https://drupal.slack.com/archives/C014QES6HSQ/p1696523077987389?thread_t...

In UI as noun it exists in forum...

core/modules/forum/src/ForumSettingsForm.php:54: 3 => $this->t('Posts - most active first'),
core/modules/forum/src/ForumSettingsForm.php:55: 4 => $this->t('Posts - least active first'),
core/modules/forum/templates/forum-list.html.twig:40:

🇺🇸United States papagrande US West Coast

This may be outdated and needs a check to see if there are still problems in D11 code. Please scan the code for uses of "post" as a noun, but ignore test fixtures.

Then spin off child issues by sub-system to keep the patches/MRs reasonably-sized.

🇺🇸United States papagrande US West Coast

@andeic, thanks for the update. I think we can close this issue and hide the useless patch.

To follow up on my empty home page alias, after deleting it I ran into other problems with the metatag module and https://www.drupal.org/project/drupal/issues/1255092 🐛 url() should return / when asked for the URL of the frontpage Needs work since the [current-page:url:relative] token no longer showed the alias. I ended up injecting the correct tag using hook_metatags_alter().

🇺🇸United States papagrande US West Coast

If the patch from https://www.drupal.org/project/drupal/issues/3059955#comment-15111076 🐛 It is possible to overflow the number of items allowed in Media Library Fixed solves this issue (and has been committed to core) then I'm closing this ticket as a duplicate.

Feel free to re-open if you disagree.

🇺🇸United States papagrande US West Coast

This happened on a 9.5.x client site after I updated to PHP 8.1. I found an old path for the home page that was missing the alias, i.e. was blank. After deleting the bad alias, the errors went away.

I'm guessing the alias has been there since Drupal 7.x and PHP 5.x and only became a problem with the upgrade to PHP 8.1.

🇺🇸United States papagrande US West Coast

While I really would like to translate the correct way, in this case I don't think we should let perfection be the enemy of the good. I agree with @ultimike that we should merge in !66.

While we wait for for https://www.drupal.org/project/drupal/issues/2546212 🐛 Entity view/form mode formatter/widget settings have no translation UI Needs review to get fixed we should split !69 into a follow up task that is postponed.

!66 will also be backwards compatible. Quite selfishly, one of my client's sites will then continue to be translated after I update the module with that fix.

🇺🇸United States papagrande US West Coast

I'm on vacation and can't look into this more at the moment, but I think @lostcarpark was on the right track in that we need to add a *.config_translation.yml file with the ajax base_route_name for the field formatter.

🇺🇸United States papagrande US West Coast

Added instructions for dealing with outdated core patches.

🇺🇸United States papagrande US West Coast

I think that works, but I'm wondering if that will be fully backwards compatible. (In my case, I don't actually have any hooks implemented.) Are there scenarios where passing $more by value will be unexpected in an existing hook?

And if this is a breaking change for existing hook implementations, does that mean the change should be reverted and a new major version released?

🇺🇸United States papagrande US West Coast

Unfortunately, the patch doesn't apply against 3.0.0-rc4.

A client is asking for a more compact UI, but I'm not sure it's worth the trouble to re-roll the patch with this issue set to Postponed.

I'm looking for overrides of form item & element margins.

🇺🇸United States papagrande US West Coast

@jpro, it's fixed in the sense that the patch has been committed and is now part of the 2.x branch.

However, you are correct that the fix hasn't been released. It looks like @opdavies created the 2.0.1 tag, but never actually released it. @opdavies, can you release a new version?

🇺🇸United States papagrande US West Coast

Closing this since we now have a stable release.

🇺🇸United States papagrande US West Coast

Triaged as part of BSI.

Is this still an issue? Can you provide steps to reproduce the problem?

🇺🇸United States papagrande US West Coast

Isn't this a duplicate of https://www.drupal.org/project/markdown/issues/3288447 📌 Drupal 10 compatibility fixes Fixed ?

🇺🇸United States papagrande US West Coast

Yes, please.

🇺🇸United States papagrande US West Coast

@godotislate, I tweaked your code to be a little easer to read, but thanks a lot for your contribution.

Production build 0.71.5 2024