Don't update log_online for certain actions

Dragooon

  • I can code! Really!
  • polygon.com has to be one of the best sites I've seen recently.
  • Posts: 1,841
Don't update log_online for certain actions
« on March 30th, 2013, 04:46 PM »
Certain actions (mostly notifications polling) shouldn't update log_online table (Subs.php, writeLog function) since it doesn't make sense as the user actually isn't online and it hogs the Who's Online area.
The way it's meant to be

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Don't update log_online for certain actions
« Reply #1, on March 30th, 2013, 04:54 PM »
There are already some actions where it doesn't, I guess notifications can be added to that list. It's... complicated but I'll look at it today.
Re: Don't update log_online for certain actions
« Reply #2, on March 30th, 2013, 06:34 PM »
OK, so I've done this, but this does have some other interesting consequences.

Someone could still be visiting the site, keeping the tab open - but not actually 'online' because their entry to who's online doesn't get renewed, though I guess if they're not actually 'online', it's all good.

Note that logging is per-action not per-subaction, so if there's any part of action=notification that needs to be logged, we need to fix that. But I'll commit what I have for this to be examined.
When we unite against a common enemy that attacks our ethos, it nurtures group solidarity. Trolls are sensational, yes, but we keep everyone honest. | Game Memorial

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: Don't update log_online for certain actions
« Reply #3, on April 3rd, 2013, 11:52 AM »
Yes, I suppose not logging these would make more sense...

A few notes that might warrant comments, though.

- I only checked your latest rev out just now (and am about to cancel a few modifications I made on my local install and was planning to commit later today or this week :^^;:), so please bear with me.

- Before checking out this topic, which I kinda remembered was floating around (and I guess it was a good idea to check it out before committing anything :P), I made some changes to fix sudden bugs in the Who's Online page that were mentioned elsewhere. The reason was actually due to the fact that when updating wedge.org to the latest rev, I forgot to update index.php as well (it's not very frequently updated, and while I usually keep an eye on its latest modification date, it looks like I forgot to do it the one time it was needed.)

- So that's the reason why there were more 'unknown action' entries... These were from both 'notifications' and 'feeds'. It's interesting to know that there are so many feed requests, BTW...!

- But call me crazy, I STILL have to ask the question: what difference does it make that a bot is downloading a feed or a page...? It's still checking out whether I have updates in some particular area (overall, board, topic, etc.) The only point in logging online data for bots is that we know what they're indexing... If they're indexing one of my feeds, I'm interested in knowing it. What do you think? Does it make sense..?

- Also, there was still a media_wo_feed entry, which should (logically) not be recorded either, if main feeds aren't...?

- I've noticed that Who.english-uk.php has a use of 'license' in one string and 'licence' in another. I've changed it to 'licence', I suppose that was the intention...?

- Also changed an 'analyzer' to 'analyser'. Is that correct? Finally, removed plenty of language strings that were actually identical to Who.english.php's...

- My guess is that you (@Arantor) started working on it, and became busy on something else, and forgot to check when you got back. It's harder to check one's code when you're committing new code rather than code updates -- I've myself often fallen in some traps because of that.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Don't update log_online for certain actions
« Reply #4, on April 3rd, 2013, 03:06 PM »
Quote
- So that's the reason why there were more 'unknown action' entries... These were from both 'notifications' and 'feeds'. It's interesting to know that there are so many feed requests, BTW...!
Given how many places it's linked, not really a surprise.
Quote
- But call me crazy, I STILL have to ask the question: what difference does it make that a bot is downloading a feed or a page...?
SMF blocked them showing up in the online log. We just inherited that behaviour.

As for whether it is meaningful, I would say it is. Given how often feeds get requested, not running additional (and not entirely cheap) queries is quite a nice thing.
Quote
- Also, there was still a media_wo_feed entry, which should (logically) not be recorded either, if main feeds aren't...?
The deal is that anything hitting action=feed will not be logged. If the media feed is handled through the feed handler it will also not be logged. If it's handled through the media area it will be logged.
Quote
- I've noticed that Who.english-uk.php has a use of 'license' in one string and 'licence' in another. I've changed it to 'licence', I suppose that was the intention...?
Yeah, I didn't check that one as heavily as I should. It's really, really, really boring to go through English US to look for differences especially when you see the alternative spellings so often.
Quote
- Also changed an 'analyzer' to 'analyser'. Is that correct? Finally, removed plenty of language strings that were actually identical to Who.english.php's.
Only in English UK. Oh and you mean the credits items. I had a feeling they were left in for a reason but I can't remember what.
Quote
became busy on something else, and forgot to check when you got back.
Pretty much.

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: Don't update log_online for certain actions
« Reply #5, on April 3rd, 2013, 04:04 PM »
Bugger... I'm currently trying out Maxthon 4 (Chrome 22 engine), and the little guy is not liking Wedge... -_-
Basically, from what I could gather: if a div is using the flex model, its children will ignore margin collapsing. i.e., my recent fix for quote margins is fucked up by Maxthon. Great...
Since my main browser is currently Sleipnir (Chrome 24), and it behaves correctly, I'd love to know if anyone noticed that issue... (It's pretty simple, post text is 'stuck' to the gray line that separates it from the post header, "Posted at ... by ...", you know.)
Quote from Arantor on April 3rd, 2013, 03:06 PM
Given how many places it's linked, not really a surprise.
Yeah... And we can't really hide these links from bots, ah ah...
Quote
As for whether it is meaningful, I would say it is. Given how often feeds get requested, not running additional (and not entirely cheap) queries is quite a nice thing.
I see...
Quote
The deal is that anything hitting action=feed will not be logged. If the media feed is handled through the feed handler it will also not be logged. If it's handled through the media area it will be logged.
Yes, it's using the media action. I'll look into it.

Oh, a suggestion for $action_no_log... Why not, instead, make this an optional third parameter in the $action_list array..?

$action_list = array(
  'action' => array('Function', 'File.php', $no_log)
);

Isn't that... 'cleaner'?
It seems technically doable, and IIRC $action_list can be altered by plugins anyway, can't it..?
The only issue is with extra_actions... It's no longer the same 'format'. But I don't see the point of this one either... Why not, simply add to $action_list..? Isn't this a global already? Yes it is...
Quote
Yeah, I didn't check that one as heavily as I should. It's really, really, really boring to go through English US to look for differences especially when you see the alternative spellings so often.
I thought you'd be using wingrep or equivalent, with a set of common used words.. No?

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Don't update log_online for certain actions
« Reply #6, on April 3rd, 2013, 04:24 PM »
Quote
Isn't that... 'cleaner'?
No it isn't, seeing that there can already be three parameters (plugin added actions will add function, file, plugin id) which means it would become 3 or 4 parameters depending on plugin.
Quote
It seems technically doable, and IIRC $action_list can be altered by plugins anyway, can't it..?
The only issue is with extra_actions... It's no longer the same 'format'. But I don't see the point of this one either... Why not, simply add to $action_list..? Isn't this a global already? Yes it is...
I explained all this when I wrote it into the plugin manager.

The first question: "Is it truly and completely sane to load a function to run a function, every page load, just to add one line to an array?" (Rinse, repeat for every plugin that adds an action.)

Of course it's not. Which is why the plugin manager can add actions directly. The whole point of plugins declaring actions up front in plugin-info.xml is so that they don't have to be hooked in the conventional sense but can be added on to the list via the point when hooks are loaded (when $settings is loaded)

That's why we don't add directly to $action_list. But if you really believe it would be better to have files loaded just to run hooks to extend the actions list, like SMF does, we can remove it. I'd really prefer you didn't, though, since it would also mean rewriting part of the plugin system (both loader and manager)

This is also why we have the third parameter in the first place. Plugins do not and cannot use loadSource which is what $action_list expects. The third parameter is the plugin id so that it knows 1) to even call loadPluginSource and 2) it has all the information it actually needs to do so.
Quote
I thought you'd be using wingrep or equivalent, with a set of common used words.. No?
Eh, Notepad++ can do that. And I did start out with that but 'licence' wasn't one of the words I checked for. That, and the fact I wanted to skim through the language files, nice to see things I don't look at very often.
Quote
(It's pretty simple, post text is 'stuck' to the gray line that separates it from the post header, "Posted at ... by ...", you know.)
Not observed in Chrome 26. I still have issues with the horizontal scrollbars though.

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: Don't update log_online for certain actions
« Reply #7, on April 3rd, 2013, 06:34 PM »
Quote from Arantor on April 3rd, 2013, 04:24 PM
No it isn't, seeing that there can already be three parameters (plugin added actions will add function, file, plugin id) which means it would become 3 or 4 parameters depending on plugin.
Hmm.
Quote from Arantor on April 3rd, 2013, 04:24 PM
I explained all this when I wrote it into the plugin manager.
But I didn't read it. I completely skipped most of the conversations about the plugin manager... I have to choose my battles ;)

Anyway, talk about clean code: if I want to prevent media feeds from being logged, instead of doing a define(), I now have to global $action_no_log, and add 'media' to it from within the feed function... :-/

I can do that if you think it's the preferred method, but I don't think it's 'clean' at all.
(Of course, adding a param to $action_list isn't gonna help in this situation, either; but I was thinking of WEDGE_NO_LOG in this case.)
Quote from Arantor on April 3rd, 2013, 04:24 PM
Not observed in Chrome 26. I still have issues with the horizontal scrollbars though.
On this very topic, with Chrome 28 Canary, at my very first post on the page, the contents of the post is limited in height to the height of my user box. Everything in the body that overflows from the assigned height then shows up on top of everything, including the following post... And it does the same for every single post after that. It's completely unreadable.

I fixed it by opening the dev tools, and disabling flex for the post area. Which totally doesn't make sense...

So, it looks like only Sleipnir (Chrome 24) seems to deal with flexbox correctly... :-/ And I don't know how to fix that.
To tell the truth, I'm very tempted to just give up on flex, even though I spent something like a week on that feature, and instead use macros to use, ahem, pure table tags by default in posts... (Which, of course, brings old problems to the table -- responsiveness and all.)

Fun, eh..?
(Oh, and yes, my font is Open Sans... The joys of custom.m1.css 8-))

PS: also, the bottom margin is bodies is eaten up in Chrome 28.
Posted: April 3rd, 2013, 06:30 PM

I'm starting to wonder if this isn't a graphic driver problem... I mean, I never had this problem with flex until today, and now I'm seeing it everywhere (in Chrome), although with different results depending on the version...
Oh, really funny: I'm in quick reply mode, and the text from my previous post is overwriting the textarea.
Let me see....                               (Oh, and yes, my font is Open Sans... The joys of custom.m1.css 8-))

I was only re-typing what was showing up at that exact position, ah ah. Well, gotta go, can't read what I'm typing anymore...

 funny shot.jpg - 67.75 kB, 560x298, viewed 157 times.


Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Don't update log_online for certain actions
« Reply #8, on April 3rd, 2013, 06:53 PM »
Quote
(Of course, adding a param to $action_list isn't gonna help in this situation, either; but I was thinking of WEDGE_NO_LOG in this case.)
WEDGE_NO_LOG had exactly the same problem, incidentally. The old check looked for existence but what happens is that existence will only occur in the directly included file. Look at the code flow - wedge_main runs, figures out the action to be diverted to, the action is loaded, THEN we do the logging, before actually heading to the action itself. This is because a lot of the time redirectexit() or obexit is called manually (which would prevent code flow returning to index.php)

So we have to do the logging before we go to do whatever the action is.

Consequently even your idea of globalling action_no_log isn't going to work because it will have already been logged by the time you get there. It's at the action level, not subaction level, that it happens. And whatever else, your file with that define simply wouldn't have been loaded.

Either move it to the main feed action (which isn't logged either, or shouldn't be IMHO) and then the no-log rule applies there, or its own action which can be excluded as you need.

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: Don't update log_online for certain actions
« Reply #9, on April 3rd, 2013, 07:12 PM »
Hmm... I guess it's a lose-lose situation, then...

I can't change the URL. That's because older installs will still need them to be valid. Or I'll have to add a redirection like I'm doing for old feeds... But that's not 'clean', either.

... :-/

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Don't update log_online for certain actions
« Reply #10, on April 3rd, 2013, 07:24 PM »
Except that if users use pretty URLs it's going to change anyway (since the PURLs scheme doesn't match what's available in any of the SMF mods)

People do expect URLs to change when software changes and it doesn't tend to upset others, I'd personally be inclined to not worry about it and just have it be whatever it is.

Nao

  • Dadman with a boy
  • Posts: 16,079

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: Don't update log_online for certain actions
« Reply #13, on April 3rd, 2013, 07:53 PM »
That's the problem.... I feel that everything should be 'natural'.
And in this particular situation, there is no way for that to happen, so I'm asking you what your preferred solution(s) are...

(Would also appreciate some feedback on Chrome + flexbox model from other users...)

godboko71

  • Fence accomplished!
  • Hello
  • Posts: 361
Thank you,
Boko