[ticket/10308] disable retain/ delete posts option when deleting a user #661

dhruvgoel92 · 2012-03-26T17:37:25Z

When deleting a user, it asks whether the posts by user should be retained
or deleted. The selection should be disable if the user has no posts.

PHPBB3-10308

imkingdavid · 2012-03-26T18:56:51Z

IMO, the option should be removed in that case, not just disabled. But either way we should explain why the option is not available to be clicked in some cases.

dhruvgoel92 · 2012-03-26T18:58:24Z

perhaps we could display NO POSTS instead of SELECT OPTION ?

naderman · 2012-04-08T20:19:18Z

I agree, it should display a text explaining that the user has no posts.

naderman · 2012-04-08T20:34:40Z

Actually another comment: In general it's a bad idea to let UI elements disappear without explanations because it will confuse users who expect to see the element there. So disabled elements are typically the better solution because they make clear that the option hasn't disappeared because of a bug, or is located somewhere else, but is in fact disabled intentionally.

p · 2012-04-09T16:51:35Z

Please research how this PR will interact with #600.

naderman · 2012-04-09T18:28:45Z

I don't think they interact, as long as poll votes always get deleted regardless of the choices in the select box.

naderman · 2012-04-10T20:44:20Z

phpBB/language/en/acp/users.php

@@ -77,6 +77,7 @@

 	'MOVE_POSTS_EXPLAIN'	=> 'Please select the forum to which you wish to move all the posts this user has made.',

+	'NO_POSTS'				=> 'The user has no posts.',


Please check if this language key already exists elswhere, I'm pretty sure it does. Please pick a unique name.

dhruvgoel92 · 2012-04-10T21:32:02Z

@naderman have fixed the language key.

naderman · 2012-04-19T21:38:33Z

phpBB/adm/style/acp_users_overview.html

@@ -140,7 +140,10 @@
 		<legend>{L_DELETE_USER}</legend>
 		<dl>
 			<dt><label for="delete_type">{L_DELETE_USER}:</label><br /><span>{L_DELETE_USER_EXPLAIN}</span></dt>
-			<dd><select id="delete_type" name="delete_type"><option class="sep" value="">{L_SELECT_OPTION}</option><option value="retain">{L_RETAIN_POSTS}</option><option value="remove">{L_DELETE_POSTS}</option></select></dd>
+			<dd><select id="delete_type" name="delete_type"><option class="sep" value="">{L_SELECT_OPTION}</option><option value="retain"<!-- IF USER_POSTS == 0 --> disabled="disabled" class="disabled-option"<!-- ENDIF -->>{L_RETAIN_POSTS}</option><option value="remove"<!-- IF USER_POSTS == 0 --> disabled="disabled" class="disabled-option"<!-- ENDIF -->>{L_DELETE_POSTS}</option></select></dd>
+			<!-- IF USER_POSTS == 0 -->


USER_POSTS does not contain the actual number of posts, but the displayed number. You can disable increasing USER_POSTS in certain forums. So this might be 0 even though the user has posts. You need to add a query to retrieve the actual number of posts in acp_users.php

dhruvgoel92 · 2012-05-09T12:12:20Z

any other improvements for this PR?

p · 2012-05-09T13:41:12Z

phpBB/includes/acp/acp_users.php

+					FROM ' . POSTS_TABLE . '
+					WHERE poster_id = '. $user_id;
+				$result = $db->sql_query_limit($sql, 1);
+				$user_row['user_has_posts'] = ($db->sql_fetchfield('post_id') ? 1 : 0);


true/false instead of 1/0?

Yeah, should probably just cast to boolean.
$user_row['user_has_posts'] = (bool) $db->sql_fetchfield('post_id');

p · 2012-05-09T13:52:00Z

Some commit messages are missing PHPBB3-10308.

p · 2012-05-09T13:52:22Z

Patch seems to work as intended.

travisbot · 2012-05-09T14:07:18Z

This pull request fails (merged 24222d3e into a618e22).

travisbot · 2012-05-09T14:18:44Z

This pull request fails (merged e9796468 into 0488d70).

dhruvgoel92 · 2012-05-09T14:19:57Z

i think i need to rebase it on the current develop-olympus right?

bantu · 2012-05-09T14:23:04Z

To fix the failing tests? No, this is a different problem. You can just ignore it for now.

dhruvgoel92 · 2012-05-09T14:24:15Z

Ohh.. okay.. i anyway rebased it on to current olympus. That wont be a problem i guess?

travisbot · 2012-05-09T14:25:56Z

This pull request fails (merged 5f6267b7 into 0488d70).

travisbot · 2012-05-09T16:09:04Z

This pull request fails (merged 8941c7c6 into 0488d70).

travisbot · 2012-05-09T16:14:26Z

This pull request fails (merged 7ac420fd into 0488d70).

travisbot · 2012-05-09T16:19:51Z

This pull request fails (merged feefce7f into 0488d70).

p · 2012-05-09T16:53:36Z

phpBB/language/en/acp/users.php

You need to amend the commits that added the newlines.

travisbot · 2012-05-09T18:21:19Z

This pull request fails (merged 4f793cb8 into 0488d70).

travisbot · 2012-05-09T18:30:21Z

This pull request fails (merged 89f19723 into 0488d70).

When deleting a user, it asks whether the posts by user should be retained or deleted. The selection should be disable if the user has no posts. PHPBB3-10308

While deletng the user, if the user has no posts in addition to the options being disabled an added message is displayed. PHPBB3-10308

Language key has been changed and has been made more specific to avoid conflicts PHPBB3-10308

language modified to be clear and select box disappears in case no posts by user. user's total posts are fetched using a new query. PHPBB3-10308

language key renamed to make its usability more clearer. PHPBB3-10308

introduces a hidden input field with retain posts as the mode in case user has no posts. PHPBB3-10308

instead of fetching all posts by user we limit the query to 1 to check if a user has posts or not PHPBB3-10308

makes user_row['user_has_posts'] boolean instead of 1 or 0. PHPBB3-10308

Language variable has be renamed for better understanding PHPBB3-10308

travisbot · 2012-05-10T17:27:41Z

This pull request fails (merged cf556f9 into 0488d70).

p-scratch · 2012-05-10T20:03:48Z

phpBB/adm/style/acp_users_overview.html

@@ -140,7 +140,12 @@
 		<legend>{L_DELETE_USER}</legend>
 		<dl>
 			<dt><label for="delete_type">{L_DELETE_USER}:</label><br /><span>{L_DELETE_USER_EXPLAIN}</span></dt>
-			<dd><select id="delete_type" name="delete_type"><option class="sep" value="">{L_SELECT_OPTION}</option><option value="retain">{L_RETAIN_POSTS}</option><option value="remove">{L_DELETE_POSTS}</option></select></dd>
+			<dd>
+			<!-- IF USER_HAS_POSTS == 0 -->


Why the == 0 check? If it's a boolean == 0 can probably be omitted.

indentation is fixed and user_posts variable is compared as a boolean variable. PHPBB3-10308

travisbot · 2012-05-10T20:22:57Z

This pull request fails (merged 041b7be into 0488d70).

dhruvgoel92 mentioned this pull request Apr 9, 2012

WIP: [ticket/9838] Delete votes on open polls, while deleting a user with his posts #600

Closed

naderman reviewed Apr 10, 2012
View reviewed changes

naderman reviewed Apr 19, 2012
View reviewed changes

p reviewed May 9, 2012
View reviewed changes

phpBB/language/en/acp/users.php

?>

Copy link

Contributor

p May 9, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to amend the commits that added the newlines.

dhruvgoel92 added 9 commits May 10, 2012 22:47

[ticket/10308] disable retain/ delete posts option when deleting a user

643970d

When deleting a user, it asks whether the posts by user should be retained or deleted. The selection should be disable if the user has no posts. PHPBB3-10308

[ticket/10308] Displays message to user if there are no posts.

239e601

While deletng the user, if the user has no posts in addition to the options being disabled an added message is displayed. PHPBB3-10308

[ticket/10308] fixes language entity.

8f7e856

Language key has been changed and has been made more specific to avoid conflicts PHPBB3-10308

[ticket/10308] fix language and user's total posts

59177a8

language modified to be clear and select box disappears in case no posts by user. user's total posts are fetched using a new query. PHPBB3-10308

[ticket/10308] renames language key to USER_NO_POSTS_DELETE

faf2322

language key renamed to make its usability more clearer. PHPBB3-10308

[ticket/10308] fixes user deletion if no posts

cf303c3

introduces a hidden input field with retain posts as the mode in case user has no posts. PHPBB3-10308

[ticket/10308] fixes sql query, limit it to 1

164054f

instead of fetching all posts by user we limit the query to 1 to check if a user has posts or not PHPBB3-10308

[ticket/10308] makes variable boolean

23ea588

makes user_row['user_has_posts'] boolean instead of 1 or 0. PHPBB3-10308

[ticket/10308] fixes language variable name

cf556f9

Language variable has be renamed for better understanding PHPBB3-10308

p-scratch reviewed May 10, 2012
View reviewed changes

[ticket/10308] fixes indentation

041b7be

indentation is fixed and user_posts variable is compared as a boolean variable. PHPBB3-10308

naderman merged commit 041b7be into phpbb:develop-olympus May 29, 2012

dhruvgoel92 deleted the ticket/10308 branch February 17, 2013 07:02

		@@ -77,6 +77,7 @@

		'MOVE_POSTS_EXPLAIN' => 'Please select the forum to which you wish to move all the posts this user has made.',

		'NO_POSTS' => 'The user has no posts.',

Uh oh!

[ticket/10308] disable retain/ delete posts option when deleting a user #661

[ticket/10308] disable retain/ delete posts option when deleting a user #661

Uh oh!

Conversation

dhruvgoel92 commented Mar 26, 2012

Uh oh!

imkingdavid commented Mar 26, 2012

Uh oh!

dhruvgoel92 commented Mar 26, 2012

Uh oh!

naderman commented Apr 8, 2012

Uh oh!

naderman commented Apr 8, 2012

Uh oh!

p commented Apr 9, 2012

Uh oh!

naderman commented Apr 9, 2012

Uh oh!

naderman Apr 10, 2012

Choose a reason for hiding this comment

Uh oh!

dhruvgoel92 commented Apr 10, 2012

Uh oh!

naderman Apr 19, 2012

Choose a reason for hiding this comment

Uh oh!

dhruvgoel92 commented May 9, 2012

Uh oh!

p May 9, 2012

Choose a reason for hiding this comment

Uh oh!

bantu May 9, 2012

Choose a reason for hiding this comment

Uh oh!

p commented May 9, 2012

Uh oh!

p commented May 9, 2012

Uh oh!

travisbot commented May 9, 2012

Uh oh!

travisbot commented May 9, 2012

Uh oh!

dhruvgoel92 commented May 9, 2012

Uh oh!

bantu commented May 9, 2012

Uh oh!

dhruvgoel92 commented May 9, 2012

Uh oh!

travisbot commented May 9, 2012

Uh oh!

travisbot commented May 9, 2012

Uh oh!

travisbot commented May 9, 2012

Uh oh!

travisbot commented May 9, 2012

Uh oh!

p May 9, 2012

Choose a reason for hiding this comment

Uh oh!

travisbot commented May 9, 2012

Uh oh!

travisbot commented May 9, 2012

Uh oh!

travisbot commented May 10, 2012

Uh oh!

p-scratch May 10, 2012

Choose a reason for hiding this comment

Uh oh!

travisbot commented May 10, 2012

Uh oh!

Uh oh!