Merge Requests

More

Recent comments

🇫🇷France fgm Paris, France

As I mentioned, if you do not actually view a node during a given day, the daily counter does not reset. It is actually the count of views on the last day there actually was a view.

Does it not reset if you actually view the node another day ? If so, can you provide a means to reproduce the issue, because this is not what I observe on my own sites.

🇫🇷France fgm Paris, France

AFAICS this means your PHP cannot see your MongoDB server.

Since your phpinfo() shows the extension is installed in your PHP, the likely culprit is the DSN you are using localhost:27017.

Did you copy and adapt the example.settings.local.php provided by the module to your actual topology ?

Try to enter the container in which you are running PHP (possibly the same one as apache) and run mongosh to check if it can indeed connect to your mongodb server. I suspect you need to use the name of the container instead of localhost.

🇫🇷France fgm Paris, France

Note that, albeit different, this is related to Provide option to replace all occurrences of a URL Postponed: needs info .

🇫🇷France fgm Paris, France

Not strictly against doing this, but it raises a number of security issues around escaping content which is an an unknown parsing context instead of being only in restricted attributes on specific elements.

Do you have a plan about how to it safely ? Consider that URLs may happen in text nodes, in HTML comments, in CDATA or PCDATA, in embedded JS, in CSS, etc. And all of those have different parsing rules.

I'm not convinced this can be done safely in all these contexts while maintaining a reasonable complexity level.

Alternatively, we might want to enable specific opt-in contexts, say for example the src attribute on link elements within the page head. Would this be appropriate for you ? If so, do you think you could imagine a UI for this ? Just a wireframe would be enough since we use the admin theme anyway. I'm not good at UI design at all.

Also, I think this could be at the global level, possibly overridable per-format, as envisioned on Need for global settings - reduce Postponed: needs info

🇫🇷France fgm Paris, France

Merged to HEAD.

🇫🇷France fgm Paris, France

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

🇫🇷France fgm Paris, France

Completed, thanks.

🇫🇷France fgm Paris, France

This version does not yet run the unit tests, AFAICS, so that needs to be added.

🇫🇷France fgm Paris, France

Fixed in today's HEAD.

🇫🇷France fgm Paris, France

fgm created an issue.

🇫🇷France fgm Paris, France

fgm created an issue.

🇫🇷France fgm Paris, France

xjm credited fgm .

🇫🇷France fgm Paris, France

Good to learn this now works.

For versions 1.x this is as designed (feature, not bug) : the statistics table accumulates views across languages and revisions, keeping only a single count per node, and will not change in 1.x for compatibility.

This is something that we probably want to change in 2.x indeed. Accordingly, converting to feature for the next version.

🇫🇷France fgm Paris, France

Potential example in the Redis module: 📌 Implement new QueueFactoryInterface Active .

🇫🇷France fgm Paris, France

Re-enabling for consideration in contrib.

🇫🇷France fgm Paris, France

Reopening for consideration in contrib Statistics 2.x

🇫🇷France fgm Paris, France

The statistics module was actually removed in Drupal 11 after being deprecated in 10.3. Moved to https://www.drupal.org/project/statistics

🇫🇷France fgm Paris, France

Questionable whether we can find a way to do this in newer versions, because of the 2-step loading, but moving to contrib just to keep it in mind.

🇫🇷France fgm Paris, France

Relevant again for the contrib version.

🇫🇷France fgm Paris, France

No such limitation for contrib statistics, so reopening.

🇫🇷France fgm Paris, France

There is no such limitation now that the module is in contrib: reopening.

🇫🇷France fgm Paris, France

This happens because core Statistics, as well as the 1.x versions of contrib Statistics only count displays in full page mode.

Enabling support for other view modes is part of the 2.x plan at 🌱 Plan for Statistics 1.x Active

🇫🇷France fgm Paris, France

Found this in the outdated core issues : wondering if the OP can provide a way to reproduce the problem on current core or the 1.0.0 contrib version on Drupal 10.3.x / 11.x alpha ?

🇫🇷France fgm Paris, France

Wow, that's a good one, actually: multi-sites can exist in multiple DBs or a single one (e.g. with Domain Access). In the case of multiple DBs there is no real issue, as there is little case to be made that those are a single site.

However within a single DB, having per-site stats could really be relevant. I'm thinking of a site factory with hundreds of sites for France Televisions, where each one had different stakeholders and would be interested in their own specific sites.

And then, of course, one would need to also provide aggregate counts for site factory stats.

🇫🇷France fgm Paris, France

This can happen if there is no view on a given node on a given day: the view count will remain unchanged. The "views today" value is actually "views on the day of the last view".

On content with enough traffic this ends up being the same because all content is seen at least once a day. To get a more accurate result you will need to add a filter ensuring that the day of the last timestamp is the same as the current day, and return 0 otherwise.

Can you check viewing the node you are interested in and then verifying that the daily count was indeed reset ?

🇫🇷France fgm Paris, France

That was not planned but it's an interesting idea: added it to the plan, thanks.

Beware that the plan is just a set of ideas for where to go: there is no timeline for actual implementation, so don't hold your breath.

🇫🇷France fgm Paris, France

added suggestion from 💬 can it count the total counts of all pages? Active

🇫🇷France fgm Paris, France

Sorry if I'm a bit dense, but I'm not sure I'm following:

I thought the finalized version would be:

1 page for the contributed version at https://www.drupal.org/docs/contributed-modules/statistics
1 page for the core D8-D10.3 version ...somewhere, containing the one currently at the contrib URL https://www.drupal.org/docs/contributed-modules/statistics/overview on a core docs URL since this is core in those versions
1 page for the core D7- version at https://www.drupal.org/docs/7/core/modules/statistics

Or alternatively/preferably:

🇫🇷France fgm Paris, France

The other thing with drush_log is that there is already a proper dependency injected $this->logger->info($message, $context); at the next line so it seems entirely redundant, does it not ?

For the array return I had forgotten to remove it. Removed.

🇫🇷France fgm Paris, France
  • If I remove the drush_log removals, this will trigger warnings in upgrade_status because that function is obsolete.
  • Undoing the changes from DefaultFactory to ContainerFactory will drop support for the ContainerFactoryPluginInterface, breaking class System changes which depend on it
  • No visible reason for the include common.inc, but since we are going for a minimal MR, this was already in, so it seems inappropriate to change it
  • The change for 9.5 is because 9.5 is the last version in the 9.x series, required for 10.0 upgrades, hence appropriate to require since this MR is about preparing for D10: anyone still on 9.x should be on 9.5.x, not 9.4, especially since this is a development tool.

I applied the other changes you requested.

🇫🇷France fgm Paris, France

Done. The MR is now significantly smaller, with less than 150 new lines, and addresses most of your comments.

🇫🇷France fgm Paris, France

We could. Or, since D9 is EOL anyway (so no one should be developing on it, hence no one will be affected), we could just accept it as is, and open one or more followups to address your comments in the original, working on D10 ? Since it doesn't work on D10 yet, no one on D10 will be affected by it not working either.

The thing is, that migration is not just about editing a few deprecations and namespace changes: some core functions were just removed without any direct equivalent, hence part of the PR size, AFAICS. Meaning restarting it for just the D10 port is likely to yield an only slightly smaller MR. I'll try to reduce its size later today, if that can help, like putting the moved function back in place and removing the CS fixes.

🇫🇷France fgm Paris, France

Yeah, my point is that even though the MR is far from ideal, it works well enough in its current state, and it is probably more useful to just merge it and open a followup for all the needed improvements on a supported core release rather than one that is EOL, because we don't know how much longer it can take to reach a good status rather than "not worse but working on supported core" which seems to be the plan here.

🇫🇷France fgm Paris, France

@joachim I updated the MR at 📌 Plans for Drupal 10 support? Needs review which unblocks the module for D10 to work on the needed updates on a supported core version.

🇫🇷France fgm Paris, France

At this point, the module passes upgrade_status checks. The only 4 remaining deprecations are internal to the module itself, unrelated to Drupal core.

There are obviously other issues, such as not using Gitlab CI, but at this point with D11 soon to be released, I think the best decision is to merge this so we can tackle the Gitlab and D11 migrations on D10, which is still relevant while D9 has been EOL for 6 months.

🇫🇷France fgm Paris, France

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

🇫🇷France fgm Paris, France

I beg to differ : when using it for automatically created tables, it's not rare for it to find errors. I don't think I've actually often used it for manually created tables, because those match their expected schema, unlike those generated by core and contrib modules.

🇫🇷France fgm Paris, France

@xmacinfo that's how I do it currently. FWIW, here is a View I'm using which you can adapt to your own needs. Beware the renaming that d.o. applied to it, and customize as you see fit.

🇫🇷France fgm Paris, France

The goal is pretty much summarized by the bit you quoted : anonymous stats. So it is definitely not an extension to GA, Matomo, and the like, as it will not rely on anything outside the site itself (no cookies, local storage, service workers, etc).

The basic idea is to focus on stats about what has been put on the screen : currently, the core version is just "nodes in full page mode" but if you look at the plan, the general idea is more about any entity in the modes chosen for counting, and possibly more (e.g. routes, routes+params) and also click-tracking, since it can actually be performed entirely anonymously.

Also, part of the idea is that the module should be a basis for third-party extensions, not trying to do everything on its own, but offering an integration point on which other services may build. A typical case for this would be stats over time, as the current core (now 1.0.0 in contrib too) only provides snapshot counts. An external service can get data from there by periodic sampling and build a history DB.

One point that has to keep in mind is that keeping stats, especially over time is costly in terms of DB writess, to an extent some users do not realize. Currently, a very coarse approximation of the cost of stats is just basically O(n) where n is the number of nodes. Now if you add click tracking it jumps to O(n^2). If you add all entity types, the multiplier for O(n^2) jumps by the number of entity types. If you add all view modes, the multiplier jumps again by the average number of bundles per entity type. And if you add history, suddenly you get to O(n^2 * s) where s is the sampling rate.

Just a typical sample on a small site with a lot of traffic: 10k nodes -> 10k*10k = 100M possible transitions in clicktracking. Keep sampling at one per minute: 1440*100M = 1.44G data points per day. Now, obviously most of these data points will be empty and will not have to be stored (assume blank = 0), but that's still a lot, with a very high maximum theoretical limit.

Most small and medium sites are not ready for this kind of DB storage. If you look at the historical issues on the core module, you'll find some users finding that just the extra load of the request rate doubling due to statistics was enough to overload their server. Hence the idea to provide an extension point so that these big data sinks can be plugged into the system instead of remaining within it.

🇫🇷France fgm Paris, France

Merged, thanks for your participation.

🇫🇷France fgm Paris, France

A View "page" display is technically not an "entity view" for Drupal, because it does not so much display the View config entity than it executes it within some context, which runs a plugin happening to be called a "page", so it is really a different thing altogether in terms of mechanism.

It also has the issue that in most cases it will actually display other (content) entities, sometimes possibly in "page" mode themselves, multiplying the view count, preventing the association of a statistics "view" to the total number of pages seen.

But that's still an interesting question indeed. Part of the "possibly any page" first item. Editing the IS accordingly.

🇫🇷France fgm Paris, France

Good that this is done. However, for future commits, it would be nice if you could use the standard d.o. commit message format ("Issue #XXXX by daffie: whatever.") so that the issue a commit addresses is automatically linked back and forth with gitlab.

Beyond that, have you though of a content update for the project page to describe that new 3.x branch ?

🇫🇷France fgm Paris, France

Makes sense, especially worrying about the BC layer only if the first steps succeed. Because it is a "nice to have", IMHO, not a "must".

🇫🇷France fgm Paris, France

The drush cr command actually invokes drupal_flush_all_caches() by way of drupal_rebuild().

See for details:

This $settings listing is incomplete. Could you paste all the memcache-related lines ?

🇫🇷France fgm Paris, France

@2dareis2do 's answer on Slack :

My patch is just a collection of other peoples commits so I am not sure I am the best qualified person to review this. This patch has been working for me though. In the meantime I have decided to restrict stats view to registered users as as well.
🇫🇷France fgm Paris, France

I would think it saner (safer ?) to introduce such a #htmx key instead of overloading the meaning of #ajax.

Then new code could opt in to the new mechanism, and the two mechanisms could coexist with the older one being deprecated and removed over time without having to maintain a compatibility layer. That seems more sustainable.

We would probably just need to error when both keys are present on an element.

🇫🇷France fgm Paris, France

Seems like this just broke the current D11 (next major) Gitlab template. See e.g. https://git.drupalcode.org/issue/htmx-3447092/-/jobs/1591833

The problem is the use of the --no-interaction CLI flag, which no longer exists in 10.5.20

🇫🇷France fgm Paris, France

FWIW tests pass normally on D11 locally. No idea why the Gitlab check fails.

🇫🇷France fgm Paris, France

fgm created an issue.

🇫🇷France fgm Paris, France

big pipe placeholders (which would be another thing to try a conversion of fairly early on

Indeed, that reminds me of another potential difficulty, which is the use of the hx-trigger="load" or "revealed" or "intersect" relying on the default browsers events or the intersection API, which is likely to interfere with our use of behaviours to support multiple loads per context like document or other node e.g. with BigPipe.

🇫🇷France fgm Paris, France

Before doing a lot with that, there's an issue with HTMX which I would not know how to solve for now : HTMX basically makes the assumption of a stable <head> section across pages, except for the title: when elements are swapped by a hx-boost, possibly inheriting if from <body hx-boost="true">, all <head> content in responses is dropped except for the <title> element.

However, Drupal pages have long been dependent on a strong relationship between <head> and <body> content, with different JS/CSS bundles from page to page, for example.

Now, you can indeed push changes to <head> using multiple response top-level element with the hx-swap-oob attribute, but that needs an ID on each modified target, which needs to pre-exist on the source page, which seems like a lot of potential trouble.

Even without using <body hx-boost="true">, partial replacement responses are often likely to require <head> changes.

I think this is something which needs to be solved earlier than later for a PoC, to outline the patterns that can be used to work around that discrepancy between the HTML model and the Drupal model.

🇫🇷France fgm Paris, France

@joachim It's strange that there are recent commit on it, then, like https://git.drupalcode.org/project/codefilter/-/commit/08f5b6ae049067de8... . But that's your call.

🇫🇷France fgm Paris, France

Needs backport to 8.x-1.x, which has the exact same problem and appears to still be maintained, so updating.

🇫🇷France fgm Paris, France

fgm created an issue.

🇫🇷France fgm Paris, France

Not sure if that is really such a good idea, because I am missing info here :on the one hand, this link appears to be the proper place for this documentation in contrib, but now D10 users (and 8, 9..) who have Statistics in core do not have any documentation left. Is this really desired ?

I thought there would be 3 doc pages: 1 for core 7, 1 for core 8 to 10.3, 1 for contrib ?

🇫🇷France fgm Paris, France

fgm created an issue.

🇫🇷France fgm Paris, France

LGTM. Preparing release.

🇫🇷France fgm Paris, France

AIUI, this was fixed by changing the definitions in .gitlab-ci.yml on 📌 Support 10.3.x in addition to 11.x Fixed .

See https://git.drupalcode.org/project/statistics/-/commit/6bcf4302d95e4a00f...

@nicxvan Unless you mean something else which remains to be done, I suggest we close this.

🇫🇷France fgm Paris, France
  • 10.3.x-dev install:
    • no statistics config
    • enable module
    • statistic.settings created, display_max_age: 3600, count_content_views: 0
    • set them to 3601 / 1
  • composer require all 11.x / drush 13
    • drush updbst:
      [notice] Module statistics has an entry in the system.schema key/value storage, but is missing from your site. 
                  
                    More information about this error
                  .
       [success] No database updates required.
       [warning] Message: Module /statistics/ has an entry in the system.schema key/value storage, but
      is missing from your site. More information about this error [1].
      
      [1] https://www.drupal.org/node/3137656
      
    • composer require drupal/statistics, get beta1
    • ls -l web/code/modules : no statistics remaining
    • drush updb throws couple of unrelated errors about missing updates layout_builder_post_update_default_expose_field_block_setting and system_post_update_sdc_uninstall
    • drush cr : no error
    • drush cget statistics/settings: still same correct config

Conclusion: all appears to be correct, as expected : there is no configuration or DB change from the core to contrib version anyway.

🇫🇷France fgm Paris, France

Added the upgrade test issue 📌 Test the core to contrib upgrade path Active . After that we should be good.

🇫🇷France fgm Paris, France

@O'Briat since you mention their roadmap, could you provide a link if they put one up ? Or copy if that's a non-confidential document you received?

🇫🇷France fgm Paris, France

Merged 9 years later. Thanks everyone.

🇫🇷France fgm Paris, France

Regarding the nid validation : the maximum check was removed for DB independence, but can we agree we should really keep the min_range => 0 check ? I do not think there is any case in which a nid can be negative, is there ?

🇫🇷France fgm Paris, France

Daniel's suggestion in #5 is relevant in the "transitions tracking" part of the plan 🌱 Plan for Statistics 1.x Active .

🇫🇷France fgm Paris, France

May be relevant again in the new module roadmap for the "transitions tracking" part of the plan at 🌱 Plan for Statistics 1.x Active .

🇫🇷France fgm Paris, France

Even better: upgrading the StatisticsStorageInterface to include a static method describing properties to send, and making that configurable, frontends could know just what to send.

This is related to the "transitions tracking" feature in the module roadmap at 🌱 Plan for Statistics 1.x Active .

🇫🇷France fgm Paris, France

Now relevant again after the core split.

Production build 0.69.0 2024