Changeset 6869

Timestamp:

10/12/2018 05:07:51 PM (7 years ago)

Author:

johnjamesjacoby

Message:

Edit Locking Improvements:

• Refactor to avoid doing unnecessary computations
• Invert default return value from false to true, requiring time to pass validation as opposed to assuming
• Improve obviousness of math computations for easier debuggability
• Update variables passed into the end return filter
• Add 6 unit tests for before/on/after, plus support for "0" as infinite
• Fix bug causing "0" values to return the opposite value
• Ensure only gmt/utc values are compared
• Add optional flag to use WordPress time instead
• Improve inline and function documentation

Fixes #3222. Props wpdennis.

Location:

trunk

Files:

• src/includes/admin/settings.php (modified) (4 diffs)
• src/includes/common/functions.php (modified) (1 diff)
• src/includes/core/options.php (modified) (1 diff)
• tests/phpunit/testcases/common/functions.php (modified) (1 diff)

trunk/src/includes/admin/settings.php

@@ -661 +661 @@
     </label>
     <label for="_bbp_edit_lock">
-        <input name="_bbp_edit_lock" id="_bbp_edit_lock" type="number" min="0" step="1" value="<?php bbp_form_option( '_bbp_edit_lock', '0' ); ?>" class="small-text"<?php bbp_maybe_admin_setting_disabled( '_bbp_edit_lock' ); ?> />
+        <input name="_bbp_edit_lock" id="_bbp_edit_lock" type="number" min="0" step="1" value="<?php bbp_form_option( '_bbp_edit_lock', '5' ); ?>" class="small-text"<?php bbp_maybe_admin_setting_disabled( '_bbp_edit_lock' ); ?> />
 
     <?php $select = ob_get_clean(); ?>
@@ -1945 +1945 @@
      * @param string $option
      * @param string $default
-     * @param bool $slug
+     * @param bool   $is_slug
+     *
+     * @return mixed
      */
-    function bbp_get_form_option( $option, $default = '', $slug = false ) {
+    function bbp_get_form_option( $option, $default = '', $is_slug = false ) {
 
         // Get the option and sanitize it
@@ -1953 +1955 @@
 
         // Slug?
-        if ( true === $slug ) {
+        if ( true === $is_slug ) {
             $value = esc_attr( apply_filters( 'editable_slug', $value ) );
 
@@ -1967 +1969 @@
 
         // Filter & return
-        return apply_filters( 'bbp_get_form_option', $value, $option );
+        return apply_filters( 'bbp_get_form_option', $value, $option, $default, $is_slug );
     }
 

trunk/src/includes/common/functions.php

@@ -191 +191 @@
  * Check a date against the length of time something can be edited.
  *
+ * It is recommended to leave $utc set to true and to work with UTC/GMT dates.
+ * Turning this off will use the WordPress offset which is likely undesirable.
+ *
  * @since 2.0.0 bbPress (r3133)
- *
- * @param string $post_date_gmt
- *
- * @return bool True if date is past, False if not
- */
-function bbp_past_edit_lock( $post_date_gmt = '' ) {
+ * @since 2.6.0 bbPress (r6868) Inverted some logic and added unit tests
+ *
+ * @param string  $datetime Gets run through strtotime()
+ * @param boolean $utc      Default true. Is the timestamp in UTC?
+ *
+ * @return bool True by default, if date is past, or editing is disabled.
+ */
+function bbp_past_edit_lock( $datetime = '', $utc = true ) {
 
     // Default value
-    $retval = false;
-
-    // Get number of minutes to allow editing for
-    $minutes = (int) get_option( '_bbp_edit_lock', 5 );
-
-    // Now
-    $cur_time = current_time( 'timestamp', true );
-
-    // Period of time
-    $lockable  = "+{$minutes} minutes";
-
-    // Add lockable time to post time
-    $lock_time = strtotime( $lockable, strtotime( $post_date_gmt ) );
+    $retval = true;
 
     // Check if date and editing is allowed
-    if ( ! empty( $post_date_gmt ) && bbp_allow_content_edit() ) {
-
-        // "0" minutes set, so allow forever
+    if ( bbp_allow_content_edit() ) {
+
+        // Get number of minutes to allow editing for
+        $minutes = bbp_get_edit_lock();
+
+        // 0 minutes means forever, so can never be past edit-lock time
         if ( 0 === $minutes ) {
-            $retval = true;
-
-        // Not "0" so compare
-        } elseif ( $cur_time >= $lock_time ) {
-            $retval = true;
+            $retval = false;
+
+        // Checking against a specific datetime
+        } elseif ( ! empty( $datetime ) ) {
+
+            // Period of time
+            $lockable = "+{$minutes} minutes";
+            if ( true === $utc ) {
+                $lockable .= " UTC";
+            }
+
+            // Now
+            $cur_time  = current_time( 'timestamp', $utc );
+
+            // Get the duration in seconds
+            $duration  = strtotime( $lockable ) - $cur_time;
+
+            // Diff the times down to seconds
+            $lock_time = strtotime( $lockable, $cur_time );
+            $past_time = strtotime( $datetime, $cur_time );
+            $diff_time = ( $lock_time - $past_time ) - $duration;
+
+            // 0 minutes set, so allow editing forever
+            if ( 0 === $minutes ) {
+                $retval = false;
+
+            // Check if less than lock time
+            } elseif ( $diff_time < $duration ) {
+                $retval = false;
+            }
         }
     }
 
     // Filter & return
-    return (bool) apply_filters( 'bbp_past_edit_lock', $retval, $cur_time, $lock_time, $post_date_gmt );
+    return (bool) apply_filters( 'bbp_past_edit_lock', $retval, $datetime, $utc );
 }
 

trunk/src/includes/core/options.php

@@ -602 +602 @@
 
 /**
+ * Output the number of minutes a topic or reply can be edited after it's
+ * published. Used by `bbp_past_edit_lock()`.
+ *
+ * @since 2.6.0 bbPress (r3246)
+ *
+ * @param bool $default Optional. Default value 5
+ */
+function bbp_edit_lock( $default = 5 ) {
+    echo bbp_get_edit_lock( $default );
+}
+    /**
+     * Return the maximum length of a title
+     *
+     * @since 2.0.0 bbPress (r3246)
+     *
+     * @param bool $default Optional. Default value 5
+     * @return int Is anonymous posting allowed?
+     */
+    function bbp_get_edit_lock( $default = 5 ) {
+
+        // Filter & return
+        return (int) apply_filters( 'bbp_get_edit_lock', (int) get_option( '_bbp_edit_lock', $default ) );
+    }
+
+/**
  * Output the group forums root parent forum id
  *

trunk/tests/phpunit/testcases/common/functions.php

@@ -634 +634 @@
 
     /**
+     * @group  locking
      * @covers ::bbp_past_edit_lock
-     * @todo   Implement test_bbp_past_edit_lock().
-     */
-    public function test_bbp_past_edit_lock() {
-        // Remove the following lines when you implement this test.
-        $this->markTestIncomplete(
-            'This test has not been implemented yet.'
-        );
+     */
+    public function test_bbp_past_edit_lock_before_5_minutes() {
+        update_option( '_bbp_edit_lock', 5 );
+        update_option( '_bbp_allow_content_edit', true );
+
+        // Before
+        $result = bbp_past_edit_lock( '4 minutes 59 seconds ago UTC' );
+        $this->assertFalse( $result );
+    }
+
+    /**
+     * @group  locking
+     * @covers ::bbp_past_edit_lock
+     */
+    public function test_bbp_past_edit_lock_on_5_minutes() {
+        update_option( '_bbp_edit_lock', 5 );
+        update_option( '_bbp_allow_content_edit', true );
+
+        // On
+        $result = bbp_past_edit_lock( '5 minutes ago UTC' );
+        $this->assertTrue( $result );
+    }
+
+    /**
+     * @group  locking
+     * @covers ::bbp_past_edit_lock
+     */
+    public function test_bbp_past_edit_lock_after_5_minutes() {
+        update_option( '_bbp_edit_lock', 5 );
+        update_option( '_bbp_allow_content_edit', true );
+
+        // After
+        $result = bbp_past_edit_lock( '5 minutes 1 second ago UTC' );
+        $this->assertTrue( $result );
+    }
+
+    /**
+     * @group  locking
+     * @covers ::bbp_past_edit_lock
+     */
+    public function test_bbp_past_edit_lock_before_0_minutes() {
+        update_option( '_bbp_edit_lock', 0 );
+        update_option( '_bbp_allow_content_edit', true );
+
+        // Before
+        $result = bbp_past_edit_lock( '4 minutes 59 seconds ago UTC' );
+        $this->assertFalse( $result );
+    }
+
+    /**
+     * @group  locking
+     * @covers ::bbp_past_edit_lock
+     */
+    public function test_bbp_past_edit_lock_on_0_minutes() {
+        update_option( '_bbp_edit_lock', 0 );
+        update_option( '_bbp_allow_content_edit', true );
+
+        // On
+        $result = bbp_past_edit_lock( '5 minutes ago UTC' );
+        $this->assertFalse( $result );
+    }
+
+    /**
+     * @group  locking
+     * @covers ::bbp_past_edit_lock
+     */
+    public function test_bbp_past_edit_lock_after_0_minutes() {
+        update_option( '_bbp_edit_lock', 0 );
+        update_option( '_bbp_allow_content_edit', true );
+
+        // After
+        $result = bbp_past_edit_lock( '5 minutes 1 second ago UTC' );
+        $this->assertFalse( $result );
     }