🇺🇸United States @MegaKeegMan

Account created on 8 July 2019, over 5 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States MegaKeegMan

I think I am also seeing this issue, and I am very confused about how to go about troubleshooting. Any advice?

🇺🇸United States MegaKeegMan

I thought the same thing and unfortunately I get the same problem. It feels like composer is just looking at the wrong thing for some reason. When I have a minute, I can try this out on a fresh install and see if I get the same thing.

🇺🇸United States MegaKeegMan

Right, but I am constraining to 1.0.x still, not 1.1.x. So it doesn't make sense to me that composer is unhappy, since it should be cool with Drupal 8, 9, or 10, and I am running into this error while my Drupal Core constraint is set to 9.5.11

🇺🇸United States MegaKeegMan

I experienced the same issue and the issue fork resolved it for me.

🇺🇸United States MegaKeegMan

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

🇺🇸United States MegaKeegMan

Just went through this with @mlncn

If you go to the media library settings at /admin/config/media/media-library
then you can enable the advanced UI. Enabling this adds a button to your media selection interface that says "Save and insert"

Otherwise, just make sure the required permission on your media library views is set to "Access media overview" instead of the default "View media"

More of an aside, but one other gotcha is that your user has to have permission to add media of the types in the field.

Once all of this is set up, you can use the media library widget to add media without having access to view the media library.

While this is great, it is all still very confusing to set up, and even still it would be great if the advanced UI was more configurable so that, for example, I could hide the "Save and select button" that doesn't do anything when your site is configured like this.

🇺🇸United States MegaKeegMan

But I acknowledge that this might be a feature for some people.

🇺🇸United States MegaKeegMan

I think this would also solve a lot of the usability challenges on this module, for what it is worth

🇺🇸United States MegaKeegMan

Okay one thing I really don't like about this implementation is that when you change the order of the deltas in the edit form, including when you delete a value, the items that you have displayed in the layout change.

🇺🇸United States MegaKeegMan

Oops I didn't realize that the changes I made affected things outside of preview mode. One more fix to make sure my index strings only show up in preview mode.

🇺🇸United States MegaKeegMan

Sorry #53 was still buggy, so ignore that one. This new one is what I meant for #53 to be. I did not make any improvements mentioned in #54. Again, this patch is for 10.3.x, but if the changes are agreeable, happy to commit them into the fork.

🇺🇸United States MegaKeegMan

Potential improvements to make on #53:

  • make indices always display, even when offset and items to display are both 0
  • Add block configuration to control whether the indices should be shown in the preview
🇺🇸United States MegaKeegMan

I went ahead and made a small change to patch #44 (I made against 10.3.x), which addresses both the bug and my need for an index to be displayed. I am happy to take any feedback, and if people like I can update the issue fork too. I pretty much just touched the code mentioned in #51.

🇺🇸United States MegaKeegMan

I can confirm that bug. Also I wanted to mention that this particular solution will be a bit difficult to use, especially in the case that there are a lot of deltas in field. I wonder if it makes sense to also display the index of each delta in the preview to make it easier to select the values you are looking for

🇺🇸United States MegaKeegMan

Thanks very much for making this happen. Very useful feature, and it looks like it is working.

🇺🇸United States MegaKeegMan

This would be super helpful. I would like this capability for taxonomy terms, but I'm sure it would be great on many other entity types.

🇺🇸United States MegaKeegMan

Hi, Ed. I apologize that this is so difficult. I have done this upgrade a handful of times, and I know that it is challenging, which is part of the reason we have not been able to streamline it. I've got a deadline approaching next week, but I would be happy to meet with you the week of the 21st. Feel free to message me vi drupal.org or slack to coordinate

🇺🇸United States MegaKeegMan

For anyone wondering, patch #283 and #284 are the same, #284 is just cleaned up with documented return types etc...

🇺🇸United States MegaKeegMan

Re-tested the migrate -> rollback -> migrate
and it looks like errors are gone and things look as they should in the database

🇺🇸United States MegaKeegMan

I tested the fix and am now able to see that I can save non-ascii characters into the value_hash column

🇺🇸United States MegaKeegMan

Ohhh I just realized that the link is under the "search and metadata" menu

🇺🇸United States MegaKeegMan

I did not mention in the original comment, but I was considering that we would not need a permission for create, given that I think someone who is allowed to edit a tracked field's parent entity should be allowed to create a value revision for that field.

🇺🇸United States MegaKeegMan

Updating the patch. Patch 2 was missing the parens on a function call. Patch 3 adds a revision timestamp and user id.

🇺🇸United States MegaKeegMan

Here is a basic patch that does what I need it to. Very happy if anyone has improvements or recommendations for improvements.

🇺🇸United States MegaKeegMan

Worked with @lelkneralfaro on this one, and we were able to sort out that creating the fields in the way I describe above does not do anything. Rather, what was missing, was that we had to

  1. go to /admin/config/search/value-revisions
  2. select the "content" check box
  3. select the field name for our regular term reference field
  4. and finally save

I was confused initially because the first time I tried this, I selected "Taxonomy term" instead of content, but what I did not understand is that selecting "Taxonomy term" at the top opens up the list of fields that are ON taxonomy terms, and not the term reference fields themselves.

Updating this issue to be about improving documentation in the UI.

🇺🇸United States MegaKeegMan

I have not yet been able to identify what might be causing this. But I will try to reproduce on a fresh installation. Thanks for looking into it.

🇺🇸United States MegaKeegMan

Tested patch on the 2.0.x branch on D10.3 and it looks good

🇺🇸United States MegaKeegMan

I created a simple patch, which should work for anyone using a mailer that incorrectly implements the mail function, like the symfony_mailer contrib module. If Your are using another mailer that correctly implements the mail function, and the mail function returns a boolean as per the Drupal core mail interface, then this patch is not for you.

With that being said, it should be pretty easy to re-patch for that, and I think that patch should also exist.

Unfortunately I don't have a list of which mailer modules do what.

🇺🇸United States MegaKeegMan

Unfortunately, that won't work in the cases where a $mail contains a boolean, which I think it should. But I'd really like to hear a maintainers thoughts on how they want to handle this, and if there is any history behind this.

🇺🇸United States MegaKeegMan

For what it's worth, the MailInterface class in Drupal core hasn't been touched for 3 years, and indicates that the mail function should return a bool, see https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

🇺🇸United States MegaKeegMan

Glad to find this issue, as I was also about to report it. It doesn't look like there is any reason this function needs to return an array, since the results are not actually used for anything besides to check if the array returned is empty. Additionally, I can see where some of the confusion is coming from. The mail function, run on line 622 of the .module, is documented to return a bool. However, the implementation that I am seeing from my current sites mailer is called in the MailManagerReplacement class of the symfony_mailer contrib module. So there seems to be a lot of confusion even deeper around whether or not we should be dealing with an array or a bool. I am not sure yet where exactly this confusion stems from, whether this a core change that contrib modules haven't caught up with or whether there are just a few implementations doing it wrong. My best guess so far is that everything should be moving in the direction of the mail function returning a bool, and then any of the code that does anything with the result of the mail function should be updated accordingly. Of course ensuring that our mailers are implementing the mail function correctly. There is also the possibility that the mail function is just documented incorrectly. If anyone has any background knowledge on this, that would be great.

🇺🇸United States MegaKeegMan

One more patch, which just includes one more line that ensures the re-send receipt action appears on the customer receipt viewing page

🇺🇸United States MegaKeegMan

And lastly one more patch. This is exactly the same as 6, but changes the title of the "View PDF" action to "Download PDF". If you do not like this change, please just work from patch 6. However, I had a complaint from a client suggesting that "View PDF" does not indicate to the user that anything will be downloaded. I think it could be argued that this usability improvement fits within the goal of making PDFs viewable by customers, per the title of the issue.

🇺🇸United States MegaKeegMan

Redoing the patch because I noticed a lot of links were broken in patch 5. This patch also adds a route to view_receipt_as_customer, and users that route to generate the link that is included in views. I am not sure if this is what would generally be desired, but it made sense to me and at the very least fulfills my needs.

🇺🇸United States MegaKeegMan

Adding a patch that replaces the admin route to the receipt tab with a non-admin route.

Not ideal yet, since the tab is still in the admin context, but otherwise seems okay for now.

🇺🇸United States MegaKeegMan

I struggled for a bit to understand how to test transactions, and I am wondering if this should be covered in this documentation or if it is covered somewhere else. The few helpful details I learned include:

  • you need to set up a sandbox app, separate from the production app, and make sure to override those values in your staging environment
  • you don't need to create a sandbox buyer account to test transactions, you can just go to the credit card generator under the top level "tools" menu item, and use credit card details from there

I was thinking these details might be helpful to include in this guide, but I did not want to edit the guide directly in case others thought that these were out of scope. In which case, maybe this page should link to some other resource that does include these details.

🇺🇸United States MegaKeegMan

Well I am actually now a little unsure if this is the same issue, but I think it is. I am having this issue only on one of my content types, and it seems to only be breaking while generating sample content for a tag reference field. It is very strange because this same field is on several other content types, but only this one content type breaks. Additionally, it doesn't matter which display I am using. Default breaks just the same as Full, which is too bad because finding this issue had my hopes up that I could just avoid using Default. And lastly, whenever I hit my exception (invalid value) a new tag actually persists, despite the problem being that the value was invalid. Every time layout builder fails to render the preview, I get a new sample tag (taxonomy term).

🇺🇸United States MegaKeegMan

@mlncn I was able to resolve that issue via a patch on https://www.drupal.org/project/drupal/issues/3087632 🐛 MenuLinkContent menu_name max length is too long Needs work
However, I believe that we are running into this issue as well

🇺🇸United States MegaKeegMan

I am working on a site that uses layout builder, and the preview on the manage layout page runs createWithSampleValues which does result in the database error:
String data, right truncated: 1406 Data too long for column 'menu_name'

I applied patch 48 and the error went away. Drupal 10, PHP 8.1

🇺🇸United States MegaKeegMan

This feature actually does work (not all of the details you mention). However, it is worth noting that you can add the receipt field to the orders view and then grant the permission to view order as well as the permission to view receipts. This way, a customer is able to view their receipts. However, like you say, the path for the receipt is under the admin path, and this is very undesirable. A customer should be able to view the receipt under a non-admin path. I would like this very much!

🇺🇸United States MegaKeegMan

Great, thanks for the tip. I will take a look at this and report back here about whether I use this or another approach.

🇺🇸United States MegaKeegMan

Changed all instances of "View display" to "View page", since all displays with a path are page displays, as far as I know. And improved my patch name.

🇺🇸United States MegaKeegMan

Updating the patch for 6.1.x-dev

🇺🇸United States MegaKeegMan

Okay tried reviewing this on 6.1.x and unfortunately does not apply

🇺🇸United States MegaKeegMan

Maybe a big ask but a token would be cool

🇺🇸United States MegaKeegMan

I am trying to use this to generate default content for a distribution though. I could write a script that gets the uuid of the admin, updates the uuid in each default content zip, and then imports. But I guess the alternative is whether the import needs to respect the uuid of the referenced user, or if it could just infer that information based on the user with the given username. Any different ideas or suggestions?

🇺🇸United States MegaKeegMan

Yes. The issue is that the uuid of the admin user is different between instances, so it tries to create a new user with the same name. So I guess I could just update the uuid to reflect the admin on the separate instance. Just a minor inconvenience.

🇺🇸United States MegaKeegMan

The "drutopia_core" issue fork needs review. Please ignore the "drutopia" issue fork (which was created before this issue was moved from the drutopia project queue).

Production build 0.71.5 2024