Jump to content

PwQuickFixes

bernhard

By bernhard,
in Modules/Plugins

 Share

Recommended Posts

bernhard Hero Member

bernhard

Posted
Posted

PwQuickFixes

This module is intended to serve as a collection of ProcessWire fixes that can easily be enabled via checkboxes in the module's config. It is NOT intended to be a replacement for github issue reports or pull requests!

The idea is that we have a place where we can put fixes for issues that are not yet fixed in the core until they get properly fixed there.

It is also not my intention to put pressure on Ryan to fix issues quicker. The idea is that we have a place where we can share our fixes across projects and across users and maybe get a better feeling on the "impact" of different issues. The module could also make it easier for Ryan to test issue reports and the solutions provided by the community:

Intended Workflow

  • Identify an issue
  • Create the related files in the fixes folder of this module
  • Create a PR so that I can merge the fix and everybody can use/test/improve it
  • Create an issue in the official PW issues repo and link the fix in your description

Usage

Just add your fixes in the fixes folder and activate them in the config of this module (via checkbox). See the Foo example module how to add JS and CSS.

img

 

https://github.com/BernhardBaumrock/PwQuickFixes

  • Like 7
Link to comment
Share on other sites
bernhard Hero Member

bernhard

Posted
  • Author
Posted

Sorry for the confusion ? 

There are several issues with the PW core. Some of them get fixed quickly, some don't. All those issues exist on every project and one has to fix them on every project. This module makes it easy to place fixes in a central place and activate them one by one. It was just a quick idea when I read this comment today: https://github.com/processwire/processwire-issues/issues/802

I've just added a fix for the annoying 404 error:

SRQ2eA4.png

https://github.com/BernhardBaumrock/PwCoreFixes/commit/c4fa410654d0d4a4f8a50c53315a350cdf60d347

  • Like 2
Link to comment
Share on other sites
bernhard Hero Member

bernhard

Posted
  • Author
Posted

Yeah, the idea is to have a shared place where we can apply those fixes and do not have to wait for core fixes. The benefit compared to placing hooks into /site/ready.php is that we can share our code, it's easier to test in different environments and we can provide better suggestions for a fix to ryan.

PS: Happy 1000 @wbmnfktr ?

  • Like 2
Link to comment
Share on other sites
adrian Hero Member

adrian

Posted
Posted
On 12/4/2019 at 6:53 AM, bernhard said:

Not sure what others think of that idea, especially @adrian @Robin S ?

Hey @bernhard - you've probably already seen this comment via Github, but I'll repost here for others. 

I like your proactive approach with module, but I fear that it might actually slow down fixing these things in the core because if users start adopting it, they'll be less likely to add their voice to the actual issue reports and so Ryan won't see that the issue affects other users as well. I hope I am wrong though and I really do appreciate your effort!

  • Like 2
Link to comment
Share on other sites
wbmnfktr Hero Member

wbmnfktr

Posted
Posted
47 minutes ago, adrian said:

I fear that it might actually slow down fixing these things in the core because if users start adopting it, they'll be less likely to add their voice to the actual issue reports

IMHO...

I see your concern and fear here but... this module isn't for the average user as they aren't that deep into the core details of ProcessWire.

Mentioning an issue is easy, reporting an issue is work, fixing an issue with this module is art.

 

  • Like 2
Link to comment
Share on other sites
szabesz Hero Member

szabesz

Posted
Posted (edited)
13 hours ago, adrian said:

Maybe it is art, but you know what would be logical and efficient ? ..... fixing issues via PRs ?

It would be, but as we all know, life is not as straightforward like that  ? There are issues Ryan solves pretty fast after being reported, there are others that take more time and there are some which seem to take forever... and the worst category is in which he is only marginally interested.

Also, what we might provide as a "fix" via a PR can easily turn out to be just a workaround and there is no point in submitting such a PR. For example, in all of my use-cases PageTable without an "add new" bottom at the TOP of the table is a UX nightmare, so I have a JS hack to clone the button from the bottom to the top. In my point of view I "fixed" this is issue, but in reality it is just a hack which would be a good fit for this module, I think.

Maybe we should call this module PwQuickFixes which might better reflect its intended purpose: temporary workarounds until they get properly fixed.

14 hours ago, adrian said:

Ryan won't see that the issue affects other users as well

We just need stats to see how many of us is using a particular quick fix, and another way to make our voice heard is to add a kind nudge by asking the user to go to the related Github issue in order to make her/his voice heard too.

Edited by szabesz
typo
  • Like 6
Link to comment
Share on other sites
wbmnfktr Hero Member

wbmnfktr

Posted
Posted
13 hours ago, adrian said:

Maybe it is art, but you know what would be logical and efficient ? ..... fixing issues via PRs

That would be a masterpiece of course!

As @szabesz already wrote... sometimes you want to take care of some fixes you need and aren't available at time.
Therefore the idea of naming this module PWQuickFixes seems to be pretty accurate.

  • Like 3
Link to comment
Share on other sites
adrian Hero Member

adrian

Posted
Posted
19 hours ago, szabesz said:

We just need stats to see how many of us is using a particular quick fix

I really like this idea and almost see it as more important than fixes that this module might contain. This is something Bernhard could implement pretty easily - whenever the module's settings are saved, it could ping a server of his with a list of which fixes are enabled. The catch of course is that some users may not want these stats tracked. Perhaps there could be an option to allow / deny tracking? It would also be great if it didn't capture any site details. Any thoughts on this @bernhard ? It would be great to show Ryan how many people some of these issues affect.

  • Like 2
Link to comment
Share on other sites
szabesz Hero Member

szabesz

Posted
Posted
1 hour ago, adrian said:

there could be an option to allow / deny tracking? It would also be great if it didn't capture any site details.

Pinging should be optional of course, and not just because of "personal preference" but also because ideally one site should only report usage once, and when the superuser switches it on/of for testing or whatever purposes, then there should be no multiple reports of using it. For this reason, I would put a another checkbox next to the "enable" one, with a label of "please report usage of this quick fix" or something like that.

Link to comment
Share on other sites
adrian Hero Member

adrian

Posted
Posted
55 minutes ago, szabesz said:

but also because ideally one site should only report usage once

I think this could be handled automatically. If this module created a unique identifier for each installation then with each module settings save the ping would update the stats for this install by passing that identifier along with the fixes that are enabled / disabled. Make sense?

  • Like 1
Link to comment
Share on other sites
adrian Hero Member

adrian

Posted
Posted

@bernhard - just submitted a PR for the childTemplates and parentTemplates removal issue (https://github.com/processwire/processwire-issues/issues/802)

https://github.com/BernhardBaumrock/PwCoreFixes/pull/1

One thing to note - I think you should perhaps set the autoload value of this module to 99999 - the reason being that Tracy's Admin Tools panel has a "Template delete" tool that allows you to delete templates that have associated pages in one step - it deletes the pages for you first and then the template. The problem is that because Tracy is set to autoload at 9999 it deletes the template before this fix is applied. The catch with loading this before Tracy is that then you can't use bd() calls to debug this module or the fixes you are writing for it, so not sure what is actually best ?

  • Like 2
Link to comment
Share on other sites
bernhard Hero Member

bernhard

Posted
  • Author
Posted

Hi guys, thx alot for the discussion ? 

On 12/5/2019 at 8:00 AM, szabesz said:

I like the idea A LOT ? However, since it is your module, you should be the one to audit – at least from the security point of view – any PRs before accepting. Will you have time for that?

Good point. I think that should not be a problem. If there is an issue with the core, it should be easier to check a simple fix than to fix the issue yourself. If it was a complicated fix, this module might not be the right place for it.

I've added @adrian as collaborator on the Repo - the goal is that this project is a "real" little community project ? 

---

On 12/5/2019 at 9:51 PM, adrian said:

I like your proactive approach with module, but I fear that it might actually slow down fixing these things in the core because if users start adopting it, they'll be less likely to add their voice to the actual issue reports and so Ryan won't see that the issue affects other users as well. I hope I am wrong though and I really do appreciate your effort!

Thx adrian, I've updated the description of the module:

Quote

Intended Workflow

  • Identify an issue
  • Create the related files in the fixes folder of this module
  • Create a PR so that I can merge the fix and everybody can use/test/improve it
  • Create an issue in the official PW issues repo and link the fix in your description

---

On 12/5/2019 at 10:41 PM, wbmnfktr said:

this module isn't for the average user as they aren't that deep into the core details of ProcessWire.

Actually I think it IS a module for the average user. See https://github.com/processwire/processwire-issues/issues/812 for example. This is a CSS issue that any webdeveloper can easily fix by some CSS rules. The only problem is: How to get the code properly into processwire. There are several possibilities:

  • Overwrite the core css file (easy, but no option, of course)
  • Create a PR (you don't know when it get's merged and you need a workaround until then)
  • Use a module like Admin Custom Files (this works for CSS and JS, but this will not work if you need hooks for your fix)
  • Add your fix via hooks in ready.php (you end up with a mess of hooks in ready.php and you need to copy over code snippets from project to project)
  • Create a module for it (that might be too much effort for most users for simple fixes)

The module makes it really easy to apply any kind of quickfix. And it makes it really easy to merge those fixes via PR's (for the ones maintaining this module).

---

On 12/5/2019 at 10:45 PM, adrian said:

Maybe it is art, but you know what would be logical and efficient ? ..... fixing issues via PRs ?

True! Nothing to add ? 

---

On 12/5/2019 at 11:06 PM, szabesz said:

but in reality it is just a hack which would be a good fit for this module, I think.

It's not intended for hacks, it's intended to fix issues that one would report in the PW issues repository until the issue get's fixed there properly. Hope that makes sense ? 

On 12/5/2019 at 11:06 PM, szabesz said:

Maybe we should call this module PwQuickFixes which might better reflect its intended purpose: temporary workarounds until they get properly fixed.

Thx, that's a good idea, thx ? 

---

On 12/6/2019 at 6:54 PM, adrian said:

I really like this idea and almost see it as more important than fixes that this module might contain. This is something Bernhard could implement pretty easily - whenever the module's settings are saved, it could ping a server of his with a list of which fixes are enabled. The catch of course is that some users may not want these stats tracked. Perhaps there could be an option to allow / deny tracking? It would also be great if it didn't capture any site details. Any thoughts on this @bernhard ? It would be great to show Ryan how many people some of these issues affect.

Nice idea. I'm not sure how helpful automatic statistics would be though. I've got bad feedback on my modules implementing google analytics download tracking and numbers where small overall anyhow, so I'm really not sure how much help a "this fix was used 5 times since 08/2019" would be ? 

What do you think of a obligatory property for every fix that links to the related PW issue report. This would make sure that the issue is reported in the issues repo, seen by Ryan and users can vote for the issue there (give a thumbs up). That would also have the benefit that users can raise their voice there, comment things, add screenshots etc.; A lot better than some strange number where nobody knows what it shows exactly...

---

On 12/7/2019 at 2:58 AM, adrian said:

One thing to note - I think you should perhaps set the autoload value of this module to 99999 - the reason being that Tracy's Admin Tools panel has a "Template delete" tool that allows you to delete templates that have associated pages in one step - it deletes the pages for you first and then the template. The problem is that because Tracy is set to autoload at 9999 it deletes the template before this fix is applied. The catch with loading this before Tracy is that then you can't use bd() calls to debug this module or the fixes you are writing for it, so not sure what is actually best ?

Could you please try to do that on your own? I've added you as collaborator ? 

---

Thx again for all your contributions! Have a great week.

  • Like 3
Link to comment
Share on other sites
matjazp Hero Member

matjazp

Posted
Posted

 

27 minutes ago, bernhard said:

Add your fix via hooks in ready.php (you end up with a mess of hooks in ready.php and you need to copy over code snippets from project to project)

But with this approach you have to install/copy the module files.

28 minutes ago, bernhard said:

It's not intended for hacks, it's intended to fix issues that one would report in the PW issues repository until the issue get's fixed there properly. Hope that makes sense

And when the fix is fixed in the core, then what? Perhaps the mandatory field for the fix (along with the title and description) should be the PW version number and fix applied only on lower versions? Also, we have AOS that provides some fixes/enchantments and Admin Actions. Where do you position PwQuickFixes? 

Link to comment
Share on other sites
bernhard Hero Member

bernhard

Posted
  • Author
Posted
2 minutes ago, matjazp said:

And when the fix is fixed in the core, then what? Perhaps the mandatory field for the fix (along with the title and description) should be the PW version number and fix applied only on lower versions? Also, we have AOS that provides some fixes/enchantments and Admin Actions. Where do you position PwQuickFixes? 

Yeah, that's something I'm not sure about yet. I thought of maybe adding a minimum PW version and a maxium PW version that makes sure that the fix is only loaded on versions between that numbers.

AOS is great, but it has lots of enhancements and it does not make it easy for anybody to contribute. PwQuickFixes should really only contain fixes of "real" issues that are reported in the pw issues repo (see the updated readme). And it should be really easy to add your own fix file, add the link to the issue report and optionally add pw minimum and maximum versions and tick a checkbox...

Link to comment
Share on other sites
  • 2 months later...
bernhard Hero Member

bernhard

Posted
  • Author
Posted

I've added a feature to define the link to the issue in the fix and also the author (github user) 

<?php namespace ProcessWire;
class Foo extends PWFix {
  public $label = "My foo label";
  public $description = "My foo description";
  public $issue = "https://github.com/processwire/processwire-issues/issues/xxx";
  public $author = "BernhardBaumrock"; // github user
  public function init() {
    // fix goes here
  }
}

D3KmgWM.png

  • Like 1
Link to comment
Share on other sites
  • 2 weeks later...
  • 1 month later...

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share
Go to topic listing

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...