some code :P

[emanuele45]

Not sure if you are interested in it (since this changes the behaviour a bit, even compared with the notes I found in the code), but I was reading the code and decided to try that approach (no idea about the implications and so on as usual ;)).

In any case, you may find useful the last commit:
emanuele45@b06ec39
that hides private topics from the feeds too.

The code is not tested much, so it may be broken, the PR is more of less to let you know the code exists if you want to consider it. ;)

[MissAllSunday]

I gotta re-write this to avoid the LEFT JOINS and avoid instantiate a new object on every request.

I swear I did cover the news and all the feed options as the client actually saw that flaw, must have been lost on some obscure branch I never pushed.

Thanks for the code.

[emanuele45]

Yep, I've seen your note in the code (don't remember where), though I like joins and hate find_in_set (both because it's limited is quite a pita in any dbms other than MySQL, because it's just emulated with "sql" functions) so I played with it. 👼

[MissAllSunday]

OK, just tested this and is always saving a -1 which means every guest would be able to see a private topic.

It also creates a row for each user selected which creates multiple rows for the same topic, it should only create 1 row per topic and all users should be appended to the existing users array.
Since the check for displaying a topic always take the first row in the privateTopics table, valid users allowed to see topics won't be able to see them since the first row would only contain -1.

[emanuele45]

OK, just tested this and is always saving a -1 which means every guest would be able to see a private topic.

Broken queries are broken... :P
My fault, forgot a piece in the queries.

It also creates a row for each user selected which creates multiple rows for the same topic, it should only create 1 row per topic and all users should be appended to the existing users array.

No, not really. It's intended...well, it's the reason of the changes I did.
Create one record per topic is what the mod does now, and then it uses find_in_set, I changed so that it creates 1 record per pair topic/user exactly to avoid the find_in_set. Then I had to introduce the -1 to avoid "confusion" between old non-private topics and new private ones.
Probably a more clean alternative would be to add a new column to topics, "is_private" int 0/1 and setting it to 1 when the topic is private, then the -1 could be omitted and the queries would have just one condition in the left join, but then would maintain the double condition in the where.

Oh well, it has been funny, thanks!

[emanuele45]

Wrong button... :P

[MissAllSunday]

Still there are a few places where the queries are broken, like the profil, recent and
There is an error on boardIndex Call to protected method PrivateTopics::doGet() and the custom message doesn't appear for those users who cannot view the last topic on a board.
There are DB errors when an user access the recent action.

I still don't get the -1, specially if ;SMF treats it as a guest, compatibility with old versions should be done on install, just a matter of porting the old data in the private_topics table to the new columns in topics table and I'm still pretty much prefer to have a single row per topic, as much as I would like to get rid of the find_in_set, there is no real alternative to it unless I take the huge task of normalize the table which of course is something I just ain't gonna do.

There is also the cache problem, some queries are cached using some users data that not necessarily is the same for another user, in that sense it would be better to not mess with the queries at all and perform all the checks after the queries are done, that way you can perform the check against a cached private topics table.

[MissAllSunday]

OK, I added the Boards and the auto suggest fixes, thanks, I wanted to fetch the news commit too but it fetches the entire PrivateTopics.xml with the find_in_set removed. Too many queries gets modified and it would be a nightmare to test all possible places.