| Index: trunk/extensions/ArticleFeedbackv5/api/ApiFlagFeedbackArticleFeedbackv5.php |
| — | — | @@ -40,29 +40,44 @@ |
| 41 | 41 | if ( $record === false || !$record->af_id ) { |
| 42 | 42 | // no-op, because this is already broken |
| 43 | 43 | $error = 'articlefeedbackv5-invalid-feedback-id'; |
| 44 | | - } elseif ( in_array( $params['flagtype'], $flags ) ) { |
| 45 | | - switch( $params['flagtype'] ) { |
| 46 | | - case 'hide': $field = 'af_is_hidden'; break; |
| 47 | | - case 'oversight': $field = 'af_needs_oversight'; break; |
| 48 | | - case 'delete': $field = 'af_is_deleted'; break; |
| 49 | | - default: return; # return error, ideally. |
| | 44 | + } elseif ( in_array( $flag, $flags ) ) { |
| | 45 | + $count = null; |
| | 46 | + switch( $flag ) { |
| | 47 | + case 'hide': |
| | 48 | + $field = 'af_is_hidden'; |
| | 49 | + $count = 'invisible'; |
| | 50 | + break; |
| | 51 | + case 'oversight': |
| | 52 | + $field = 'af_needs_oversight'; |
| | 53 | + $count = 'needsoversight'; |
| | 54 | + case 'delete': |
| | 55 | + $field = 'af_is_deleted'; |
| | 56 | + $count = 'deleted'; |
| | 57 | + default: return; # return error, ideally. |
| 50 | 58 | } |
| 51 | 59 | |
| 52 | 60 | if( $direction == 'increase' ) { |
| 53 | 61 | $update[] = "$field = TRUE"; |
| 54 | 62 | } else { |
| 55 | 63 | $update[] = "$field = FALSE"; |
| 56 | | - } |
| 57 | | - } elseif ( in_array( $params['flagtype'], $counters ) ) { |
| | 64 | + } |
| | 65 | + |
| | 66 | + // Increment or decrement whichever flag is being set. |
| | 67 | + $counts[$direction][] = $count; |
| | 68 | + // If this is hiding/deleting, decrement the visible count. |
| | 69 | + if( ( $count == 'hide' || $count == 'deleted' ) |
| | 70 | + && $direction == 'increase' ) { |
| | 71 | + $counts['decrease'][] = 'visible'; |
| | 72 | + } |
| | 73 | + } elseif ( in_array( $flag, $counters ) ) { |
| 58 | 74 | // Probably this doesn't need validation, since the API |
| 59 | 75 | // will handle it, but if it's getting interpolated into |
| 60 | 76 | // the SQL, I'm really wary not re-validating it. |
| 61 | | - $field = 'af_' . $params['flagtype'] . '_count'; |
| | 77 | + $field = 'af_' . $flag . '_count'; |
| 62 | 78 | |
| 63 | 79 | // Add another where condition to confirm that |
| 64 | 80 | // the new flag value is at or above 0 (we use |
| 65 | 81 | // unsigned ints, so negatives cause errors. |
| 66 | | - |
| 67 | 82 | if( $direction == 'increase' ) { |
| 68 | 83 | $update[] = "$field = $field + 1"; |
| 69 | 84 | // If this is already less than 0, |
| — | — | @@ -78,69 +93,65 @@ |
| 79 | 94 | // Decrementing from 0 is not allowed. |
| 80 | 95 | $where[] = "$field > 0"; |
| 81 | 96 | } |
| 82 | | - } else { |
| 83 | | - $error = 'articlefeedbackv5-invalid-feedback-flag'; |
| 84 | | - } |
| 85 | 97 | |
| 86 | | - // Newly abusive record |
| 87 | | - if ( $flag == 'abuse' && $record->af_abuse_count == 0 ) { |
| 88 | | - $counts['increment'][] = 'abusive'; |
| 89 | | - } |
| | 98 | + // Adding a new abuse flag: abusive++ |
| | 99 | + if( $flag == 'abuse' && $direction == 'increase' |
| | 100 | + && $record->af_abuse_count == 0 ) { |
| | 101 | + $counts['increment'][] = 'abusive'; |
| | 102 | + } |
| | 103 | + // Removing the last abuse flag: abusive-- |
| | 104 | + if( $flag == 'abuse' && $direction == 'decrease' |
| | 105 | + && $record->af_abuse_count == 1 ) { |
| | 106 | + $counts['decrement'][] = 'abusive'; |
| | 107 | + } |
| 90 | 108 | |
| 91 | | - if ( $flag == 'oversight' ) { |
| 92 | | - $counts['increment'][] = 'needsoversight'; |
| 93 | | - } |
| 94 | | - if ( $flag == 'unoversight' ) { |
| 95 | | - $counts['decrement'][] = 'needsoversight'; |
| 96 | | - } |
| | 109 | + // note that a net helpfulness of 0 is neither helpful nor unhelpful |
| | 110 | + $netHelpfulness = $record->af_helpful_count - $record->af_unhelpful_count; |
| 97 | 111 | |
| | 112 | + // increase helpful OR decrease unhelpful |
| | 113 | + if( ( ($flag == 'helpful' && $direction == 'increase' ) |
| | 114 | + || ($flag == 'unhelpful' && $direction == 'decrease' ) ) |
| | 115 | + ) { |
| | 116 | + // net was -1: no longer unhelpful |
| | 117 | + if( $netHelpfulness == -1 ) { |
| | 118 | + $counts['decrement'] = 'unhelpful'; |
| | 119 | + } |
| 98 | 120 | |
| 99 | | - // Newly hidden record |
| 100 | | - if ( $flag == 'hide' && $record->af_is_hidden == 0 ) { |
| 101 | | - $counts['increment'][] = 'invisible'; |
| 102 | | - $counts['decrement'][] = 'visible'; |
| 103 | | - } |
| 104 | | - // Unhidden record |
| 105 | | - if ( $flag == 'unhide' ) { |
| 106 | | - $counts['increment'][] = 'visible'; |
| 107 | | - $counts['decrement'][] = 'invisible'; |
| 108 | | - } |
| | 121 | + // net was 0: now helpful |
| | 122 | + if( $netHelpfulness == -1 ) { |
| | 123 | + $counts['increment'] = 'helpful'; |
| | 124 | + } |
| | 125 | + } |
| 109 | 126 | |
| 110 | | - // Newly deleted record |
| 111 | | - if ( $flag == 'delete' && $record->af_is_deleted == 0 ) { |
| 112 | | - $counts['increment'][] = 'deleted'; |
| 113 | | - $counts['decrement'][] = 'visible'; |
| 114 | | - } |
| 115 | | - // Undeleted record |
| 116 | | - if ( $flag == 'undelete' ) { |
| 117 | | - $counts['increment'][] = 'visible'; |
| 118 | | - $counts['decrement'][] = 'deleted'; |
| 119 | | - } |
| | 127 | + // increase unhelpful OR decrease unhelpful |
| | 128 | + if( ( ($flag == 'unhelpful' && $direction == 'increase' ) |
| | 129 | + || ($flag == 'helpful' && $direction == 'decrease' ) ) |
| | 130 | + ) { |
| | 131 | + // net was 1: no longer helpful |
| | 132 | + if( $netHelpfulness == 1 ) { |
| | 133 | + $counts['decrement'] = 'helpful'; |
| | 134 | + } |
| 120 | 135 | |
| 121 | | - // Newly helpful record |
| 122 | | - if ( $flag == 'helpful' && $record->af_helpful_count == 0 ) { |
| 123 | | - $counts['increment'][] = 'helpful'; |
| | 136 | + // net was 0: now unhelpful |
| | 137 | + if( $netHelpfulness == 0 ) { |
| | 138 | + $counts['increment'] = 'unhelpful'; |
| | 139 | + } |
| | 140 | + } |
| | 141 | + } else { |
| | 142 | + $error = 'articlefeedbackv5-invalid-feedback-flag'; |
| 124 | 143 | } |
| 125 | | - // Newly unhelpful record (IE, unhelpful has overtaken helpful) |
| 126 | | - if ( $flag == 'unhelpful' |
| 127 | | - && ( ( $record->af_helpful_count - $record->af_unhelpful_count ) == 1 ) ) { |
| 128 | | - $counts['decrement'][] = 'helpful'; |
| 129 | | - } |
| 130 | 144 | |
| 131 | 145 | if ( !$error ) { |
| 132 | | - $dbw = wfGetDB( DB_MASTER ); |
| 133 | | - $dbw->update( |
| | 146 | + $dbw = wfGetDB( DB_MASTER ); |
| | 147 | + $success = $dbw->update( |
| 134 | 148 | 'aft_article_feedback', |
| 135 | 149 | $update, |
| 136 | 150 | $where, |
| 137 | 151 | __METHOD__ |
| 138 | 152 | ); |
| 139 | 153 | |
| 140 | | - if( $direction == 'decrease') { |
| 141 | | - // This is backwards to account for a users' unflagging something. |
| 142 | | - ApiArticleFeedbackv5Utils::incrementFilterCounts( $pageId, $counts['decrement'] ); |
| 143 | | - ApiArticleFeedbackv5Utils::decrementFilterCounts( $pageId, $counts['increment'] ); |
| 144 | | - } else { |
| | 154 | + // If the query worked, update the filter count rollups. |
| | 155 | + if( $success ) { |
| 145 | 156 | ApiArticleFeedbackv5Utils::incrementFilterCounts( $pageId, $counts['increment'] ); |
| 146 | 157 | ApiArticleFeedbackv5Utils::decrementFilterCounts( $pageId, $counts['decrement'] ); |
| 147 | 158 | } |
| Index: trunk/extensions/ArticleFeedbackv5/api/ApiViewFeedbackArticleFeedbackv5.php |
| — | — | @@ -268,7 +268,7 @@ |
| 269 | 269 | break; |
| 270 | 270 | case 'visible': |
| 271 | 271 | $where['af_is_deleted'] = 0; |
| 272 | | - $where['af_is_hidden'] = 0; |
| | 272 | + $where['af_is_hidden'] = 0; |
| 273 | 273 | break; |
| 274 | 274 | case 'invisible': |
| 275 | 275 | $where[] = 'af_is_hidden > 0'; |