Jump to content

Mike Rockett

  • Content Count

  • Joined

  • Last visited

  • Days Won


Everything posted by Mike Rockett

  1. Great stuff - and does the language support hook-setup work as expected? You're quite right indeed, will sort that out some time tomorrow. πŸ™‚
  2. I actually see I did that already πŸ˜…So feel free to leave out the static method call above and let me know if it works.
  3. So I've been a bit busy this morning refactoring and brooming. A man is busy of late, but hey, gotta make some time for these things eventually. Anyways, couple of things I've done so far: Updated the underlying sitemap package. Refactor: split some code out into concerns, added support helpers, added return types and fixed up doc blocks. Dropped support for PW 2.8 and PHP < 7.1 (should be 7.2, but fine for now) Manually pulled in the merge request with an additional static method to add the language support hooks. I don't really have a good test case to work with, but this is the idea: use MarkupSitemap; wire()->addHookAfter(MarkupSitemap::getAdditionalPages, function ($event) { $page = $event->arguments(0); $language = $event->arguments(1); MarkupSitemap::applyLanguageSupportHooks(); // Add additional pages }); It should internally track whether or not they've been added, but I've simply not tested it yet, so can't say for sure. πŸ˜…That aside though, surely I can add the hooks directly in ___getAdditionalPages? If you'd like to give it a spin, please check out the develop branch directly.
  4. Thanks so much @d'HinnisdaΓ«l, really appreciate the effort here πŸ™‚ Will review, test and merge this morning. Regarding videos, you might actually want to hold off on that for a bit, as I'd first like to add the additional sub-element class for it, and perhaps a few others as well. The Sitemap class itself comes from ThePixelDeveloper over at GitHub, and so I think I might want to upgrade that instead, to utilise the newer features / code-base.
  5. @teppo – Are you keen on putting together a merge request for this or would you like me to give it a bash?
  6. Note to self: When creating tags, be sure to actually click on "Create Tag" 🀣 Please try again πŸ™‚
  7. Have release 1.5.59 with these changes. πŸ™‚
  8. @adrian – I'd be happy to. I see from a previous discussion with Teppo that the priority was set due to a conflict with sitemaps. That being said, I do believe there is a measure in place to circumvent sitemap.xml URIs from being logged, so as a result, I don't see why we can't change the hook and priority. At the end of the day, it kinda makes sense. Modules that need to do things with 404 events should be given the opportunity to do so before JL comes in as a last resort. So I'm going to go ahead and do this – if anyone encounters any issues, we'll need to deal with them on an as-and-when basis.
  9. Hi guys, Super sorry for not getting back to you – much like @kongondo, royally swamped (this time of year). First off, I'm battling to understand why folks are having install/upgrade troubles here. Whilst JL1 doesn't use a full-on DB migration strategy, it does have an iterative cycle. If you're on schema v4 and upgrade to the latest version, it should be doing v5 and then v6. Only thing I can think of then is that the schema version got messed up somehow. It gets saved in the module’s config, so I really don't see what could have messed it up (I recently formatted those files, but naturally that should not make a difference). I'd need to try reproduce this to see what might be going on. As an aside, JL2 will use DB migrations, so this issue would less likely be encountered unless the applicable tracking table was truncated. Hoping to get JL2 wrapped up over the holidays (lots of leave-days, no big plans). @adrian – I see PagePathHistory doesn't set its own priority, and JL does. I recall doing this for a reason, but given what you've said, perhaps JL should be somehow setup to be the 'last resort'? @teppo – Given the schema errors that are being experienced, I think that for the time being, I'm going to simply truncate the string that gets stored to 512 characters. Also a little confused by the 404 logger – it really should not be doing anything if turned off. I do see that it only logs the 404 if there are no jumplinks – I'll definitely fix up that part. Update: I've pushed out 1.5.58 - please let me know if the DB errors stop. Thanks!
  10. Hey @teppo and @d'HinnisdaΓ«l – both very good ideas indeed. I'm happy to implement both hooks, the only problem is that this time of year is super busy for me, so I won't be able to get to it straight away. If it's something you're looking for in the immediate-term, please feel free to pop me a merge request or two. In the meantime, I'll open up an issue on the repo. πŸ™‚
  11. You're welcome. JL2 (when I get there πŸ™ˆ) will give some more control here. This case would be covered by subscribe/{path} --> @[1495]/{path}, and then there'd either be a {query} wildcard or an option on the jumplink itself to automatically carry it over.
  12. Any time πŸ™‚ I've rolled out 1.5.57 with a fix, so you should be good to go without the {!all}, which is somewhat unsafe IMO.
  13. @adrian Ok so this seems to be the wildcard cleaner. Because {all} is the equivalent of .*, it includes the query string in this case and therefore cleans it like a normal wildcard. The only option for the time being is to disable cleaning of {all}, by changing it to {!all} in the destination. I know it's not perfect, but JL doesn't actually handle query strings very well – they should actually be handled separately somehow. Edit: I'm actually going to see if I can update the cleaner to prevent query string character cleaning.
  14. Ah of course - didn't click that it would be the same as the previous query πŸ™ˆ πŸ˜† Ok I'm able to reproduce, will investigate
  15. Going to have to spin this up and see what's going on. Mind sharing your source/destination with me? Sorry, I edited that post as you were replying
  16. @adrian I wonder if database->escape_string is playing a role here? I think I'm doing it differently in JL2, but that's nowhere near done in terms of a frontend. πŸ™ˆ Scratch that, it's being used in the importer. Long day I tell you
  17. Hey @adrian Super odd, that. Will take a look.
  18. @adrian β€” Glad you're all sorted there πŸ™‚
  19. @adrian Page identifiers were never intended to be used as part of a destination. These identifiers use the full, absolute URL of the page being identified. Perhaps destination selectors could come in handy here? Haven't ever tried it this way before, but I imagine redirecting subscribe/{all} to [[1495]]{all} (or [[1495]]/{all}} if trailing slashes are turned off) would work. Anything inside the square braces is passed to pages->get and replaced with page->url, which is why I believe this will work.
  20. Thanks @teppo – have reviewed and merged in with some minor UI/textual adjustments to the config. Tested it too, works great! πŸ™‚
  21. @teppo Quick follow-up on your merge request, unless you'd like me to take care of it? πŸ™‚
  22. @apeisa I think I'll take the trimming option here, for now. I have been doing incremental work on JL2 in my very short-lived spare-time, and will be using a text column for referrers in the hits table. I'm deciding whether or not the 404 monitor should still be a separate module like I'd originally intended - still makes more sense to me, but either way, the same column type will be used there.
  23. @Ben Sayers This could indicate that something's wrong with your blog setup. To clarify, are you deleting posts or un-publishing them? Either way, whatever is going on here is preventing the 404 event from kicking in, which means Jumplinks has nothing to do. When it comes to common patterns like this (ie, "blog/*"), the only things that come to mind are urlSegments and custom htaccess directives, both of which I suspect are not the case for you. Worst case, I would try reproducing on a fresh install. On a side note: I don't know ProCache very well, but could it possibly be that?
  24. Ah ok, thanks for the clarification there. Guess the optimist in me was hanging around πŸ™‚ Good to know - thanks!
  25. @wbmnfktr - Hence why I'm needing some more info – not sure if any additional composer support has been added. Would be great if I could trigger an install during module install from the UI. πŸ™‚
  • Create New...