New revs - Public comments

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: New revs - Public comments
« Reply #15, on August 1st, 2011, 04:54 PM »
Quote from Arantor on August 1st, 2011, 04:34 PM
No... the original code had an in_array to select only the regions of interest, and IIRC there was an isset for something else at the end before actually adding it to the loop but off hand I can't remember what.
Nope, at least nothing you committed. Rev 897 is where I made the change, and it's just basically the addition of array_flip and isset, and the removal of in_array and continue. (continue is okay but if there's only going to be one line after it, might as well skip it... :^^;:)
Quote
Right now am dealing with the fact that the imperative scheduled system is completely broken (and in fact I'm dubious about how it ever worked when I tested it originally because it shouldn't have done),
I never tested it, myself. (I mostly test my own additions, as for the rest I'm hoping our beta testers do some work, I have my hands full :P)
Quote
will commit that when I've fixed it and finished the feature that actually uses it.
'kay!

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #16, on August 1st, 2011, 05:03 PM »
I never tested it that thoroughly either seeing as I had nothing to plug into it but I'm adding the guts of my redirection topics mod so that you can set moved notices to disappear automatically after a few days.

I haven't yet decided about making moved notices auto redirect, since that could get nasty if not done properly. (There is no actual reliable way to know if a topic is a moved notice. Yes it has an icon and a given subject but the icon is not checked during the post routine...)
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

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #18, on August 1st, 2011, 05:39 PM »
We're already storing one of those for deleted posts. I guess we could make the inference that if it's got a previous board id and isn't in the recycle bin, it's a moved topic.

That makes sense until you realise you can move topics in and out of the recycle bin freely if you have the relevant permissions.

Nao

  • Dadman with a boy
  • Posts: 16,079

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #20, on August 1st, 2011, 10:04 PM »
Yes, when we figure out how we're going to handle deleted-but-not-really-deleted posts.
Re: New revs - Public comments
« Reply #21, on August 1st, 2011, 10:16 PM »
Quote
* I thought it would be amusing if the redirection topic strings actually used number_context()... (index.language.php, MoveTopic.template.php)
I was in two minds about that. I basically stole the code from my own mod there for that part, didn't see too much point reinventing the wheel.

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: New revs - Public comments
« Reply #22, on August 1st, 2011, 10:34 PM »
Quote from Arantor on August 1st, 2011, 10:16 PM
I was in two minds about that. I basically stole the code from my own mod there for that part, didn't see too much point reinventing the wheel.
The only point is to have only two strings for the day list, as opposed to a dozen :)

Re: recycle bin, I thought we'd agreed (although it was long ago...) that we ccould simply do it like here: a Hide (Unapprove) button that would set posts to status!=1 or something... Then if you hide a post, you get the ability to either restore it (Publish), or restore it and set its posting date to the current time (Publish to date). Which is practical on a blog when you want to publish in intervals. (Only works if nothing was posted after the message.)

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #23, on August 1st, 2011, 10:38 PM »
Quote
The only point is to have only two strings for the day list, as opposed to a dozen
*facepalm* Yes, good plan.
Quote
Re: recycle bin, I thought we'd agreed (although it was long ago...) that we ccould simply do it like here: a Hide (Unapprove) button that would set posts to status!=1 or something...
That wasn't the plan from what I remember; we didn't really get into any conclusion as to what we wanted to do,   just getting rid of the existing bin setup for something cleaner.

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: New revs - Public comments
« Reply #24, on August 1st, 2011, 11:11 PM »
Quote from Arantor on August 1st, 2011, 10:38 PM
Quote
The only point is to have only two strings for the day list, as opposed to a dozen
*facepalm* Yes, good plan.
Lol, don't worry, I stopped counting long ago the number of times I had some obvious action to take and I did it the complicated way. I guess we all sometimes need some perspective from the thing we're right into. ;)
Quote
That wasn't the plan from what I remember;
Oh, hamburgers!
Quote
we didn't really get into any conclusion as to what we wanted to do,   just getting rid of the existing bin setup for something cleaner.
Yeah, I remember that what bothered me the most was the inability to get a id != 2 for the recycle bin. Since it was enabled by default at install time, it's always a hardcoded number. That's my first reason for using the moderation system for post deletion. Other reasons: (1) UI is more natural to user if they can restore from the original topic, (2) no more bullshit with multiple deleted posts in a single topic resulting in multiple topics in the recycle bin, etc... It's all handled logically if we use post statuses to delete them.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #25, on August 1st, 2011, 11:13 PM »
Quote
It's all handled logically if we use post statuses to delete them.
On a technical level we can solve it like that, but we need to handle the UI a little differently; to me, the concept of using unapproval to actually 'delete' doesn't seem to sit quite right with me.

Nao

  • Dadman with a boy
  • Posts: 16,079

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #27, on August 1st, 2011, 11:49 PM »
I think we should keep unapproved separate from deleted, not only for semantic reasons, but also because access to recycling is a different permission to access to (own/any) moderated posts.

Not only would you filter them differently, you'd probably have different UI for the different types of posts; if you delete something, you're indicating that you want it out of the way, as opposed to something not yet made live.

I'd also note that I don't particularly like the idea of unapproved posts/topics being used for future-dated/blog type posts, I'd rather keep them out of the loop (keeps the main table leaner which always helps in performance terms)

Nao

  • Dadman with a boy
  • Posts: 16,079
Re: New revs - Public comments
« Reply #28, on August 2nd, 2011, 12:00 AM »
Pseudo-deleted doesn't have to have the same internal status id as not-yet-approved.

I don't really see how it could hurt performance as long as we have a serious index on the status field...?

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #29, on August 2nd, 2011, 12:08 AM »
Quote
I don't really see how it could hurt performance as long as we have a serious index on the status field...?
This is where it gets complicated. As I discovered when testing with large datasets, I discovered that having everything in a single master table actually does hurt performance because you just can't make use of the indexes the ideal way - by segmenting by board, you get to instantly drop a decent percentage out of the index.

500m topics in a single board when that's all I had, really hurt compared to 1.5m topics split by three boards.

The more that can be kept out of the main table, or at least efficiently indexed the better. If we have two columns filtering the status, the index probably won't be that efficient - it would probably be better if we were to index on a single column with all four (approved/not approved/deleted/not deleted) states in a single column, it would probably index better, but benchmarking will tell us for sure.