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

Soft delete #1017

Merged
merged 209 commits into from Jul 13, 2013

Conversation

Projects
None yet

jellydoughnut and others added some commits Jun 18, 2010

[feature/soft-delete] Lay the groundwork for a soft-delete feature
So far, I've added no new functionality.  The biggest change here is adjusting the DB column names to "visibility" rather than "approved".  Some things here are pretty likely to change, for example the name and location of the topic_visibility class. Happy birthday phpBB :)

PHPBB3-9657
[feature/soft-delete] Correct some mistakes in e8d47
Notably: Uncomment the die() in create_schema_files, and add the class that makes everything tick.

PHPBB3-9657
[feature/soft-delete] I told you I was going to rename the class!
Rename topic_visibility class to phpbb_visibility. Also a bit of work to the class itself, mostly cleanup and adding the comments that I'd previously written.

PHPBB3-9657
[feature/soft-delete] Implement the ability to soft-delete and restor…
…e posts

The soft delete feature seems to work.  Tests are pending.  A real icon is pending.  Add the permissions and the interface to soft-delete posts.  Also able to restore posts via the MCP queue

PHPBB3-9657
[feature/soft-delete] Add unit tests for the phpbb_visibility class
Add unit tests for the phpbb_visibility class.  Adjust the phpbb_visibility class to pass those unit tests.  The changes are pretty small, actually.

PHPBB3-9657
[feature/soft-delete] Rename phpbb_visibility class to phpbb_content_…
…visibility

Rename the class to more accurately reflect what it does.

PHPBB3-9657
[feature/soft-delete] Add a processor for action == restore in mcp_qu…
…eue.php

Restoring a post within mcp_queue.php didn't do anything before this commit.  Now it does, by way of a function which is very similar to approve_post.

PHPBB3-9657
[feature/soft-delete] Fix some small bugs
To wit: using non-existing constant POST_DELETED in posting.php; first test
post was initially unapproved; soft delete checkbox appeared at post time
Links pointing to the wrong place.

PHPBB3-9657
[feature/soft-delete] Remove imageset/ which was created by merge con…
…flict

We don't have imagesets in 3.1 anymore.

PHPBB3-9657
[feature/soft-delete] Update restoring feature to use ajax if requested.
Also fixes the mcp as a hole:
- displayes a success message
- gives a link to the post, if only one was restored

PHPBB3-9657
[feature/soft-delete] Simplification part2: user can see all item vis…
…ibilities

If the user can see all visibilities, we can simply leave out the query part,
instead of adding a bunch of ANDs.

PHPBB3-9657
[feature/soft-delete] Add get_visibility_sql_forums based on global
The resulting query is 4-times faster, as the forum_id IN () arrays are
smaller and we need less AND/OR to build the hole query. The main difference
between those two functions is, that this one takes an array of included ids and
the _global one takes an array of excluded ids.

PHPBB3-9657
[feature/soft-delete] Try to fix search.php
at least it's running now, but the performance is not very good.

PHPBB3-9657
[feature/soft-delete] Update search to use $post_visibility
Todo: Sphinx currently does not respect this setting at all.

PHPBB3-9657
[feature/soft-delete] Comment out user_posts update for the moment
It should rely on the permissions of the post not the current user.

PHPBB3-9657
else
{
include_once($phpbb_root_path . 'includes/functions_display.' . $phpEx);
include_once($phpbb_root_path . 'includes/functions_display.' . $phpEx);

This comment has been minimized.

Copy link
@EXreaction

EXreaction Jul 11, 2013

Contributor

include_once

@@ -354,6 +372,7 @@
'TOPIC_LOCKED_SUCCESS' => 'The selected topic has been locked.',
'TOPIC_MOVED_SUCCESS' => 'The selected topic has been moved successfully.',
'TOPIC_NOT_EXIST' => 'The topic you selected does not exist.',
'TOPIC_RESTORED_SUCCESS' => 'This topic has been restored successfully.',

This comment has been minimized.

Copy link
@EXreaction

EXreaction Jul 12, 2013

Contributor

"The selected topic..." is what everything else uses, may want to use the same wording

'DELETE_POSTS_CONFIRM' => 'Are you sure you want to delete these posts?',
'DELETE_POSTS_PERMANENTLY_CONFIRM' => 'Are you sure you want to <strong>permanently</strong> delete these posts?',
'DELETE_REASON' => 'Soft delete reason',
'DELETE_REASON_EXPLAIN' => 'The reason is only shown to moderators when post is soft deleted.',

This comment has been minimized.

Copy link
@EXreaction

EXreaction Jul 12, 2013

Contributor

"The specified reason for deletion will be visible to moderators."

@nickvergessen

This comment has been minimized.

Copy link
Contributor Author

nickvergessen commented Jul 12, 2013

Updated everything

@@ -422,9 +423,10 @@

This comment has been minimized.

Copy link
@EXreaction

EXreaction Jul 12, 2013

Contributor

Always true

This comment has been minimized.

Copy link
@nickvergessen

nickvergessen Jul 12, 2013

Author Contributor

Fixed

$topic_last_read = (isset($topic_tracking_info[$topic_id])) ? $topic_tracking_info[$topic_id] : 0;
$sql = 'SELECT post_id, topic_id, forum_id
FROM ' . POSTS_TABLE . "
WHERE topic_id = $topic_id
" . (($auth->acl_get('m_approve', $forum_id)) ? '' : 'AND post_approved = 1') . "

This comment has been minimized.

Copy link
@EXreaction

EXreaction Jul 12, 2013

Contributor

Always true

This comment has been minimized.

Copy link
@nickvergessen

nickvergessen Jul 12, 2013

Author Contributor

Fixed

@@ -174,7 +175,7 @@
// The FROM-Order is quite important here, else t.* columns can not be correctly bound.
if ($post_id)

This comment has been minimized.

Copy link
@EXreaction

EXreaction Jul 12, 2013

Contributor

Always true

This comment has been minimized.

Copy link
@nickvergessen

nickvergessen Jul 12, 2013

Author Contributor

No?

@@ -297,8 +305,7 @@
}
$topic_id = (int) $topic_data['topic_id'];
//
$topic_replies = ($auth->acl_get('m_approve', $forum_id)) ? $topic_data['topic_replies_real'] : $topic_data['topic_replies'];
$topic_replies = $phpbb_content_visibility->get_count('topic_posts', $topic_data, $forum_id) - 1;

This comment has been minimized.

Copy link
@EXreaction

EXreaction Jul 12, 2013

Contributor

Always true

@@ -609,6 +612,7 @@
'REPLY_IMG' => ($topic_data['forum_status'] == ITEM_LOCKED || $topic_data['topic_status'] == ITEM_LOCKED) ? $user->img('button_topic_locked', 'TOPIC_LOCKED') : $user->img('button_topic_reply', 'REPLY_TO_TOPIC'),
'EDIT_IMG' => $user->img('icon_post_edit', 'EDIT_POST'),
'DELETE_IMG' => $user->img('icon_post_delete', 'DELETE_POST'),

This comment has been minimized.

Copy link
@EXreaction

EXreaction Jul 12, 2013

Contributor

Always true

{
$has_attachments = true;
}
}
$rowset[$row['post_id']] = array(
'hide_post' => ($row['foe'] && ($view != 'show' || $post_id != $row['post_id'])) ? true : false,
'hide_post' => (($row['foe'] || $row['post_visibility'] == ITEM_DELETED) && ($view != 'show' || $post_id != $row['post_id'])) ? true : false,
'post_id' => $row['post_id'],
'post_time' => $row['post_time'],

This comment has been minimized.

Copy link
@EXreaction

EXreaction Jul 12, 2013

Contributor

Always true

@EXreaction

This comment has been minimized.

Copy link
Contributor

EXreaction commented Jul 12, 2013

Please make a test that checks visibility after moving/merging/copying topics

@nickvergessen

This comment has been minimized.

Copy link
Contributor Author

nickvergessen commented Jul 12, 2013

@EXreaction the last bunch of your "Always true" comments is off, I guess that happened when I pushed my changes (but already fixed all of the appearances)

nickvergessen added some commits Jul 12, 2013

Merge remote-tracking branch 'phpbb/develop' into ticket/9657
* phpbb/develop: (216 commits)
  [ticket/11626] Remove last reference to template in ldap
  [ticket/11626] Remove LDAP dependency on template
  [develop-olympus] Increment version number to 3.0.13-dev.
  [develop-olympus] Add changelog for 3.0.12 release.
  [develop-olympus] Bump version numbers for 3.0.12-RC1 release.
  [develop-olympus] Bumping version numbers to final for 3.0.12 releases.
  [ticket/11669] Fix PHP bug #55124 (recursive mkdir on /./)
  [ticket/11668] Run lint test at the end of the test suite
  [ticket/11548] Fix test errors in groups test on develop
  [ticket/11548] Check upload avatar URL the same way as in phpBB 3.0
  [ticket/11548] Fix incorrect usage of array_map on acp groups page
  [ticket/11665] Fix test class name
  [ticket/11664] Stop creating php.html file in root path in tests
  [ticket/11665] Can't change file names already sent to set_filenames
  [ticket/11662] Typos: occured -> occurred
  [ticket/11662] Typos: occured -> occurred
  [ticket/11660] Fix bugs from bugs in #11651 (missing vars, db->sql_connect)
  [feature/auth-refactor] Add parent::setUp() in setUp()
  [feature/auth-refactor] Changes
  [feature/auth-refactor] DataProvider for acp_board test
  ...

EXreaction added a commit that referenced this pull request Jul 13, 2013

@EXreaction EXreaction merged commit 87795eb into phpbb:develop Jul 13, 2013

1 check passed

default The Travis CI build passed
Details

@nickvergessen nickvergessen deleted the nickvergessen:feature/softdelete-1-permission branch Apr 10, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.