[ticket/11522] Bad topic count per forum with shadow topic #2020

Closed
wants to merge 2 commits into
from

Projects

None yet

6 participants

@Zoddo
Contributor
Zoddo commented Feb 8, 2014

This patch added a verification to know if a shadow topic is deleted during a topic move and changed the topic count of the forum if necessary.

http://tracker.phpbb.com/browse/PHPBB3-11522

@Zoddo Zoddo [ticket/11522] Bad topic count per forum with shadow topic
This patch added a verification to know if a shadow topic is deleted during a topic move and changed the topic count of the forum if necessary.

PHPBB3-11522
9dcbcb5
@bantu
Member
bantu commented Feb 8, 2014

Replaces #2019, see my comments there.

@Zoddo Zoddo and 1 other commented on an outdated diff Feb 8, 2014
phpBB/includes/functions_admin.php
@@ -519,6 +519,36 @@ function move_topics($topic_ids, $forum_id, $auto_sync = true)
WHERE ' . $db->sql_in_set('topic_moved_id', $topic_ids) . '
AND forum_id = ' . $forum_id;
$db->sql_query($sql);
+
+ if ($affected_rows = $db->sql_affectedrows())
+ {
+ switch ($db->sql_layer)
@Zoddo
Zoddo Feb 8, 2014 Contributor

@bantu has written : "Did you copy and paste this block from anywhere?" on #2019

Yes, I copy and paste this code from set_config_count()

@bantu
bantu Feb 8, 2014 Member

These columns are integer columns, you can do maths with them regardless of the DBMS (default switch case). The workaround in set_config_count() is required because these values are stored in a varchar column.

@Zoddo
Zoddo Feb 8, 2014 Contributor

@bantu has written : "Should use proper abstraction instead of switching on DBMS type." on #2019

I had not found any function in order to do it in dbal.

@bantu
bantu Feb 8, 2014 Member

Yes, this does not exist in develop-olympus.

@bantu bantu and 1 other commented on an outdated diff Feb 8, 2014
phpBB/includes/functions_admin.php
+ break;
+
+ case 'postgres':
+ // Need to cast to text first for PostgreSQL 7.x
+ $sql_update = 'CAST(CAST(forum_topics::text as DECIMAL(255, 0)) - ' . (int) $affected_rows . ' as VARCHAR(255))';
+ $sql_update_real = 'CAST(CAST(forum_topics_real::text as DECIMAL(255, 0)) - ' . (int) $affected_rows . ' as VARCHAR(255))';
+ break;
+
+ // MySQL, SQlite, mssql, mssql_odbc, oracle
+ default:
+ $sql_update = 'forum_topics - ' . (int) $affected_rows;
+ $sql_update_real = 'forum_topics_real - ' . (int) $affected_rows;
+ break;
+ }
+
+ $sql = 'UPDATE ' . FORUMS_TABLE . '
@bantu
bantu Feb 8, 2014 Member

There is a call to sync() at the end of this function. Please explain how this query is correctly placed in this function, say in constrast to changing something in sync().

@Zoddo
Zoddo Feb 8, 2014 Contributor

Because $auto_sync is set to false in mcp_main.php on line 659.
The sync() function is not called.

@nickvergessen
Contributor

This needs a new PR for develop
@Zoddo if you can create one for develop based on your olympus patch, that would be awesome.
If you don't know how to do that, you can either join our IRC channel #phpbb-dev on freenode.net, or ask someone else to do it ;)

@nickvergessen nickvergessen added this to the 3.0.13-RC1 milestone Mar 14, 2014
@Zoddo
Contributor
Zoddo commented Mar 29, 2014

Sorry, I don't seen your post.

PR #2200 created

@nickvergessen nickvergessen removed this from the 3.0.13-RC1 milestone Aug 22, 2014
@Nicofuma
Member

@Zoddo Bump

@nickvergessen
Contributor

This should also have tests then.
@Nicofuma want to take it over?

@Nicofuma Nicofuma self-assigned this Sep 19, 2014
@Nicofuma
Member

I will the next week

@nickvergessen nickvergessen added this to the 3.0.13 milestone Oct 29, 2014
@bantu bantu removed this from the 3.0.13 milestone Jan 20, 2015
@nickvergessen nickvergessen added this to the 3.0.14 milestone Feb 2, 2015
@marc1706
Member

@Nicofuma bump

@Galixte
Contributor
Galixte commented Apr 1, 2015

Hi,

any news about that ?

@Zoddo Zoddo closed this Oct 10, 2015
@Zoddo Zoddo deleted the Zoddo:ticket/11522 branch Oct 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment