q2a / question2answer

Question2Answer is a free and open source platform for Q&A sites, running on PHP/MySQL.
http://www.question2answer.org/
GNU General Public License v3.0
1.63k stars 628 forks source link

DOHIDEALL button on user profile page. #926

Open ihlassovbetov opened 2 years ago

ihlassovbetov commented 2 years ago

I noticed that "dohideall" button appears on users profile page even if the user does not have any visible posts. Logically, the button should not be displayed if the user does not have any visible questions. This can be fixed with function qa_db_get_user_visible_postids($userid) and with implementing if statement on result of that function.

pupi1985 commented 2 years ago

The requirement makes sense (assuming you meant does not have any visible posts).

The implementation you suggest, however, is inefficient. If a user has X visible posts, you would be getting from the database to the memory X post ids just to check if there is at least 1, each time a user profile page is displayed to a Moderator+. It would make more sense to only check if there is at least one. That would only require a single boolean and will not hit the database so hard.

Considering you have development experience, do you think you could provide a pull request for this? Officially this should go to dev but I believe if the change is small enough it could go to bugfix.

ihlassovbetov commented 2 years ago

yes, i meant posts. And the above function is retries all data of users visible posts, so as you said it is redundant. So, if it is okay, I can write a new function that counts if any visible posts and returns true/false boolean.

pupi1985 commented 2 years ago

Exactly. Something like qa_db_user_has_visible_posts($userid). It could use and EXISTS clause or maybe just a similar query with a LIMIT 1. Either of them will stop the execution of the query and return as fast as the condition is met (or proven false).

ihlassovbetov commented 2 years ago

is this okay?

function qa_db_count_user_visible_postids($userid){
    if (qa_to_override(__FUNCTION__)) { $args=func_get_args(); return qa_call_override(__FUNCTION__, $args); }
    $count = qa_db_read_one_value(qa_db_query_sub(
        'SELECT COUNT(*) FROM ^posts WHERE userid = # AND type IN ('Q', 'A', 'C', 'Q_QUEUED', 'A_QUEUED', 'C_QUEUED')',
        $userid
    ), true);

    return $count > 0;
}
pupi1985 commented 2 years ago

Just a few comments:

The fact that the function returns one value (such as a COUNT(*)) doesn't mean it will run faster. In fact, once it counts the first post it will then count the second, and so on. The idea is to stop counting after the first one is found. So the LIMIT approach I've mentioned above would be this:

SELECT 1 FROM ^posts
WHERE userid = $ AND type IN ('Q', 'A', 'C', 'Q_QUEUED', 'A_QUEUED', 'C_QUEUED')
LIMIT 1

BTW, userid = # is semantically incorrect. It should be userid = $. The core is wrong.

If the function is called count_user_visible_postids then I would expect an integer to be returned. However, a boolean is returned. The boolean is enough (and faster).

The comparison of the SELECT 1 could just check if the result is null or not.

I'd say there's no need for the qa_to_override.

Those are all the things I can think of :)

ihlassovbetov commented 2 years ago

ok it makes sense now, and should queued posts be considered as visible ones or not ?

pupi1985 commented 2 years ago

Considering visible posts are not hidden posts, then they are visible.