Account created on 20 September 2008, almost 16 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States generalredneck

As an FYI, any new release of drupal/core will create drupal/core-recommended with requirements of symfony at ~v6.4.8. So when 10.3.0 comes out for example (or a new beta), and someone is using drupal/core-recommended, a work around will be to use drupal/core instead and then pin the packages until this gets fixed upstream at simplesaml.

🇺🇸United States generalredneck

Truth... no. It's heavy handed. You can use one of the hooks to limit what data is stored. I'll sleep on it and work up a suggestion until I get you some 3.0 features worked out. Ideally you would only index the items that are referenced in views config. But at the moment you get every possible item that could be used.

🇺🇸United States generalredneck

@paraderojether,

So, as a heads up, This module is using the default Gitlab CI for Drupal template. You can see it's installation as part of the repository, and the instructions used to install it can be found here: https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr ....

That said, the phpcs.xml.dist used by the default CI is located here: https://git.drupalcode.org/project/gitlab_templates/-/blob/main/assets/p...

The command you used to check coding standards would not be equivalent to what was checked by CI, but goes above and beyond adding the DrupalPractice standards and additional file types.

While I appreciate any contributions that correct coding standards, Charles has satisfied my criteria for marking this task as complete. I'll leave the issue open for a few more days, as if you wish to receive credit for this issue, you will make the code changes you suggest as a Merge Request. If you wish, you may even add a custom phpcs.xml.dist that holds this module to the higher standards as a value add and ensure that Gitlab CI passes.

Thanks

🇺🇸United States generalredneck

@clarkssquared,

So, as a heads up, This module is using the default Gitlab CI for Drupal template. You can see it's installation as part of the repository, and the instructions used to install it can be found here: https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... .

That said, the phpcs.xml.dist used by the default CI is located here: https://git.drupalcode.org/project/gitlab_templates/-/blob/main/assets/p...

The command you used to check coding standards would not be equivalent to what was checked by CI, but goes above and beyond adding the DrupalPractice standards and additional file types.

While I appreciate any contributions that correct coding standards, Charles has satisfied my criteria for marking this task as complete. I'll leave the issue open for a few more days, as if you wish to receive credit for this issue, you will make the code changes you suggest need to be made as a Merge Request. If you wish, you may even add a custom phpcs.xml.dist that holds this module to the higher standards as a value add and ensure that Gitlab CI passes.

Thanks

🇺🇸United States generalredneck

Odd. CI is running clean. Can you verify paraderojether, you are against the latest commit?

🇺🇸United States generalredneck

Functionally works on Drupal 11

🇺🇸United States generalredneck

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

🇺🇸United States generalredneck

That works now, Broken out fixes into

📌 Fix PHP Coding standards Active
📌 Fix Javascript Coding standards issues Active

🇺🇸United States generalredneck

While you are in there fix the cspell issue. webportal can be refactored into a different function name that has a valid English word. See https://git.drupalcode.org/project/datetime_now/-/jobs/1806925

🇺🇸United States generalredneck

Thank you all for your patience and work!

🇺🇸United States generalredneck

This is likely not ideal. All other implementations like login_emailusername or ldap all use a form alter to achieve building out the form. I wanted to see what happened if I told the emailtfaloginform to have the same id as the form it's extending, and it seems to be working just fine. I increased the module weight just in case to accommodate for making sure it comes after the user module.

Thoughts?

🇺🇸United States generalredneck

There are great work-arounds here. I suspect that several people got to this situation after mismanaging configuration using configuration export. It appears that's what happened on a site I'm maintaining because looking at the key_value table, the update clearly ran at some point.

🇺🇸United States generalredneck

I contacted chr.fritsch and martin_klima via their contact forms. The message read as follows:

Hey there

I know not every maintainer gets notified about new issues added to their
issue queue for every project, so I thought I would reach out to you to see
if you would be interested on commenting on
https://www.drupal.org/project/access_unpublished/issues/3451657 💬 Offering to maintain Access Unpublished Active where I'm
offering my help maintaining the module "Access Unpublished". I would love
to see if I could get your response on the issue to help move it's
maintenance and issue queue forward.

I'm also contacting the other two maintainers on the project.

Let me know what you think and if I can help out. Feel free to reach to to me
via slack too if you wish.

Thanks,
Allan Chappell
GR

I was unable to reach aberg as their contact tab is turned off.

🇺🇸United States generalredneck

I can confirm this issue. Same is true when integrating with Login Email or Username . That module no longer takes affect.

🇺🇸United States generalredneck

Merging what we have for now. Will look into what else needs to happen.

🇺🇸United States generalredneck

This looks good to me, do we have a proposed start date?

🇺🇸United States generalredneck

Just stating for the record, I never received a reply in slack.

🇺🇸United States generalredneck

Thanks for your contribution.

🇺🇸United States generalredneck

Markuz_cz,
Ok, so I have a feel for your level of experience. I don't know if you have ever "patched" a module before, but that's essentially what I'm asking you to do to test it out. Here are some instructions on how to do it both manually if you downloaded the module, or via composer if you are installing your modules that way. Given that, I would really expect you to do this on a copy of a website (not your production one) so that if anything goes wrong we can just trash it.

So if you aren't "naturally sorting" anything (which in the case of the image you aren't for sure) then having this installed shouldn't hurt anything beyond being a draw on performance, but it does hook in to "entity save", so basically any time you save anything like configuration, content, blocks, etc.

Lastly, I'm just pointing out that I think you have in fact pointed out a bug. I'm fairly certain I can reproduce it on a clean install of Drupal, but I need to do some more testing before I'm comfortable with releasing the fix. It's so wildly different that this may become a new 3.0 version.

🇺🇸United States generalredneck

I sent a message via DM in slack to see if I could rouse an answer. Here's the message:

🇺🇸United States generalredneck

I'm pretty sure this is related to 🐛 Notice: Undefined index: value in Drupal\views\Plugin\views\filter\NumericFilter->acceptExposedInput() Needs work . It may possible be a duplicate? though I landed here because I"m getting a similar issue even after upgrading to 10.2.5 where the prior fix was released.

🇺🇸United States generalredneck

Follow up task to fix coding standards.

🇺🇸United States generalredneck

That's a new file... png based on the diff.

diff --git a/core/tests/fixtures/files/image-test-iccp-profile.png b/core/tests/fixtures/files/image-test-iccp-profile.png
new file mode 100644
index 0000000000000000000000000000000000000000..29cb8945f8e4c1ae40de24bf888715e4d870a164
--- /dev/null
+++ b/core/tests/fixtures/files/image-test-iccp-profile.png
@@ -0,0 +1,24 @@
+‰PNG
🇺🇸United States generalredneck

So as an FYI, this module is about "SQL sorting" and not Solr sorting. They are two very different implementations. This is not a bug report, but rather a feature request. Given the status of this module, as "feature complete" I'll leave this issue open so others can see the solution as described by #4, but as of now, I have no intention of adding this capability as it's my belief that this is a space for a completely different module.

If someone feels froggy enough to contribute it, I will be here to review, though I think it would be better suited as a separate project.

🇺🇸United States generalredneck

I imported the view used in the automated test. I don't think this patch fixes the full problem. I used a modified version of the test's content and put in the title of I'm testing special characters day&night© ® € ™ ← ↑ → ↓ ♠ ♣ ♥ ♦. It really is just the Ampersand (&) that causes issues. While the "title" is fixed in the view, the other fields that rendered are not.

Here is the full source code of the view outputed by the test. Note that the link, description, dc:creator, and guid> fields are all double encoding the ampersand.


<?xml version="1.0" encoding="utf-8"?>
<rss xmlns:dc="http://purl.org/dc/elements/1.1/" version="2.0" xml:base="http://drupal-10.lndo.site/">
  <channel>
    <title>Rss</title>
    <link>http://drupal-10.lndo.site/</link>
    <description/>
    <language>en</language>
    
    <item>
  <title>Hey Hey</title>
  <link>http://drupal-10.lndo.site/Hey%20%3Csup%3EHey%3C/sup%3E</link>
  <description>Hey &lt;sup&gt;Hey&lt;/sup&gt;</description>
  <pubDate>Hey Hey</pubDate>
    <dc:creator>Hey <sup>Hey</sup></dc:creator>
    <guid isPermaLink="true">http://drupal-10.lndo.site/Hey%20%3Csup%3EHey%3C/sup%3E</guid>
    </item>
<item>
  <title>I'm testing special characters day&amp;night© ® € ™ ← ↑ → ↓ ♠ ♣ ♥ ♦</title>
  <link>http://drupal-10.lndo.site/I%26#039;m testing special characters day&amp;amp;night© ® € ™ ← ↑ → ↓ ♠ ♣ ♥ ♦</link>
  <description>I'm testing special characters day&amp;amp;night© ® € ™ ← ↑ → ↓ ♠ ♣ ♥ ♦</description>
  <pubDate>I&amp;#039;m testing special characters day&amp;amp;night© ® € ™ ← ↑ → ↓ ♠ ♣ ♥ ♦</pubDate>
    <dc:creator>I'm testing special characters day&amp;night© ® € ™ ← ↑ → ↓ ♠ ♣ ♥ ♦</dc:creator>
    <guid isPermaLink="true">http://drupal-10.lndo.site/I%26#039;m testing special characters day&amp;amp;night© ® € ™ ← ↑ → ↓ ♠ ♣ ♥ ♦</guid>
    </item>

  </channel>
</rss>
🇺🇸United States generalredneck

So I was trying to reproduce this issue. It seems that there's another issue at play here that actually seems to render the code in the if statement moot anyway because we are checking for $this->options['settings']['link_to_entity'] === TRUE.

The problem here is that $this->options['settings']['link_to_entity'] is in fact the number 1 instead of the boolean TRUE

Given that, we need to fix that check so this can be reproducible. Given that, Immediately looking at the code I can see that prior to this fix, $alter['rendered']['#title']['#context']['value'] was always created and always a string or Markup object because of $this->htmlTitleFilter->decodeToMarkup() always returns that type. With this fix, that array item doesn't exist and "may" cause unexpected side effects, but I don't know that for certain. It also seems like this would only ever happen if for what ever reason the "title" was empty some how from upstream.

No amount of me messing around with a node and view settings in the UI got me an empty value there. So I have to assume that this has to be on some other type of entity and possibly a broken view configuration?

Given that, after fixing the check in the patch, I'm not receiving any bad side affects. I would feel much more comfortable with steps to reproduce this and/or a view this happens on for us to inspect. However, given the realtivly simple nature of this fix, I'll go ahead and mark it RTBC

🇺🇸United States generalredneck

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

🇺🇸United States generalredneck

@quadrexdev,

Aren't you a busy bee today. I'll go through all your stuff in the next couple of days.

🇺🇸United States generalredneck

What a good looking patch you got there. Functionality, Reviewed and tested by the community

🇺🇸United States generalredneck

I some how missed getting this one in. let me see if it still applies

🇺🇸United States generalredneck

My mistake, I had existing configuration that was causing the issue, reverting my status change. 

🇺🇸United States generalredneck

Using the code verbatim results in:

In DiscoveryTrait.php line 53:
                                                                                                                                                                                                                                                               
  The "entity:delete_action:advertiser" plugin does not exist. Valid plugin IDs for Drupal\Core\Action\ActionManager are: comment_unpublish_by_keyword_action, node_make_sticky_action, node_unpromote_action, node_assign_owner_action, node_promote  
  _action, node_unpublish_by_keyword_action, node_make_unsticky_action, user_remove_role_action, user_cancel_user_action, user_add_role_action, user_unblock_user_action, user_block_user_action, entity:publish_action:block_content, entity:publish_action:  
  comment, entity:publish_action:menu_link_content, entity:publish_action:node, entity:publish_action:path_alias, entity:publish_action:taxonomy_term, action_message_action, action_goto_action, entity:delete_action:comment, entity:delete_action:node, en  
  tity:save_action:block_content, entity:save_action:comment, entity:save_action:file, entity:save_action:menu_link_content, entity:save_action:node, entity:save_action:taxonomy_term, entity:save_action:user, entity:unpublish_action:block_content, entit  
  y:unpublish_action:comment, entity:unpublish_action:menu_link_content, entity:unpublish_action:node, entity:unpublish_action:path_alias, entity:unpublish_action:taxonomy_term, action_send_email_action      
🇺🇸United States generalredneck

one last thing... have you tried this? Computed Field . It might give you what you want.

🇺🇸United States generalredneck

Actually, I guess it depends...

Like if you can compute the value ahead of time, you can add custom items to the Sort Index. Check out the views_natural_sort.api.php file... I think you might find something in there.

But if you are looking for something on the fly... like based off a views configuration... that's going to be quite a bit more difficult as the module relies on a prebuilt index to do the computations...

Example.. removing all the "a, an, the"s from words... or the special stuff we do with numbers to make sure they sort 1, 2, 10... instead of 1, 10, 2. These are all strings stored in the database and linked to the field that they are doing magic on. See the views_natural_sort table in the database.

But say you wanted to rewrite and sort based off of an argument passed in... that couldn't be easily done without knowing what all the possible arguments could be ahead of time.

🇺🇸United States generalredneck

@plato1123
I got this rerolled for you. See if you can give it a look now. Either check out/download the MR or pull in the latest dev release and apply the patch from the "plain diff" link above.

🇺🇸United States generalredneck

Rerolled this MR with the latest Dev release.

🇺🇸United States generalredneck

Rerolling this Merge Request with what's in the current dev release since there were a massive amount of code cleanup changes.

🇺🇸United States generalredneck

I maintained backward compatibility using the old-school function_exists methodology. This ensures compatibility with Drupal 8-11.

🇺🇸United States generalredneck

Apaderno,
Take your time, I'm not in a hurry to get this one, I want to give the maintainer every benefit of the doubt, I'm not looking to "take this away" but I am looking to make sure it stays maintained. It's likely that the maintainer doesn't have email notifications turned on for new issues created in the issue queue. So if you need to take a couple of days or week or what ever, as long as expectations are set, I think due diligence has been done.

🇺🇸United States generalredneck

So I'm leaving the last PHPstan error for another ticket. It's addressed by 📌 Automated Drupal 11 compatibility fixes for views_natural_sort Needs review . Thanks so much for your work.

🇺🇸United States generalredneck

As indicated by the PHPStan run, this only fixes some of the errors. We had a similar fix marked in 🐛 Deprecation errors Fixed but for different pieces. I'm going to update the merge request here with the other undefined property fixes.

🇺🇸United States generalredneck

As indicated by the PHPStan run, this fixes one of the errors. I'm going to mark this as a duplicate of 🐛 Deprecated function: Creation of dynamic property Needs review but give credit to both of yall for reporting, testing, and commiting the fix for this instance. Both this issue and the one I'm pointing at are caught by the latest PHPStan implementation in the Gitlab CI template. See https://git.drupalcode.org/issue/views_natural_sort-3445353/-/jobs/1523402

🇺🇸United States generalredneck

Moving over to the Project Ownership issue queue for administrator review and contact of the owner of Datetime Now . It appears draenen was active on Drupal.org as little as 4 days ago according to their list of posts .

🇺🇸United States generalredneck

Yep. Check out the #flysystem channel in drupal slack. And also chexkout 🌱 Planning for Release 3.0.0, Drupal 10.1+ Compatibility, Flysystem v3.1 Compatibility Needs work this has been a pretty long standing issue. I haven't had the time to tackle it.

🇺🇸United States generalredneck

Grevil,

I see where you are coming from. Most of us who have experianced this tried to use the configuration form after the bug. This essentially wiped the configuration.

Obviously I hadn't thought of those who still have configuration intact... say upgrading form beta6 to -dev

🇺🇸United States generalredneck

Everything is part of the merge request. You can get the patch or the full code from the links up above. If toy are using beta8 it appears the patch may not apply cleanly at the moment

🇺🇸United States generalredneck

It IS possible to handle warnings, and therefore forego suppressing them all, but it would require setting up an error handler to do it. That said, if we are worried about particular image profiles, is there a way to whitelist some and check to see if the image is what we want? Is that too process intensive?

Trying to throw some fresh ideas at this that I didn't see in the comments. I do like Niklan's thoughts though.

🇺🇸United States generalredneck

Nope still hanging out here waiting for feedback. I'm taking a guess noone has tested my work. I'll come back to this at some point this quarter.

🇺🇸United States generalredneck

@markus_cz,

can you try this MR? This SHOULD decouple the "natural" sort plugin from the normal string fields and add a new proto field. You should now be able to search for "naturally sorted" in the fields to find the ones that can be sorted naturally. So this breaks it up so that the options are no longer "ascending, descending, ascending Naturally, and descending naturally" but instead just "ascending and descending" always. This is the first step on a way to removing some headaches for me as a maintainer as well, but it's a pretty big usability change...

All that to say though, that if you have any fields that are using the "natural" plugin by the scene and marked as just regular sort orders instead of their natural counter parts, this should switch the plugin over to the "standard" one and remove the dependency on the module, allowing you to uninstall this module.

🇺🇸United States generalredneck

Sorry about the long hiatus. Right now, with this patch, it's indexing "everything". It's not quite feature parity with the D7 version. I'm working my way out of a hole here a bit, and I hope that it soon will actually be something like:

Only the fields that have views that have natural sorting applied to them will be indexed. So yall haven't missed anything I don't think, it's just not there.

🇺🇸United States generalredneck

Fabinasierra,

I see you are frusturated. By the definition of fixed in the D.O Documentation this issue is a part of the dev release.

I do hear your concern though, and I think what you are trying to say is that there wasn't a tagged release fixing this issue in an easy way someone doing a composer update and get the fix without having to go find it in the dev version.

With that I can only say I'm sorry to have caused you problems. I will try to get an additional release out to hopefully ease the frustration of others. I'm sorry it wasn't there to help you.

Production build 0.69.0 2024