Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOHIDEALL button on user profile page. #926

Open
ihlassovbetov opened this issue Dec 10, 2021 · 7 comments
Open

DOHIDEALL button on user profile page. #926

ihlassovbetov opened this issue Dec 10, 2021 · 7 comments

Comments

@ihlassovbetov
Copy link

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

ihlassovbetov commented Dec 10, 2021

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
Copy link
Contributor

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
Copy link
Author

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

@pupi1985
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants