Changeset 6869

Timestamp:

10/12/2018 05:07:51 PM (8 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 );
         }