Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.5
    • Component/s: Filters
    • Labels:

      Description

      After having legacy filters and locations deprecated since 2.2 (see MDL-29995 and release notes), it's time to drop support for them for 2.3.

      This issue will take rid of them. Ciao

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Eloy Lafuente (stronk7) added a comment -

            Note: this will require to check all the custom filters we have published and/or are being used in sites like moodle.org to be sure they have been updated.

            Show
            Eloy Lafuente (stronk7) added a comment - Note: this will require to check all the custom filters we have published and/or are being used in sites like moodle.org to be sure they have been updated.
            Hide
            Michael de Raadt added a comment -

            Hi, Eloy.

            It's deprecation time. This is the last outstanding deprecation issue from 2.3.

            Could you take care of this deprecation, or provide more detail so a member of the STABLE team can do it?

            Show
            Michael de Raadt added a comment - Hi, Eloy. It's deprecation time. This is the last outstanding deprecation issue from 2.3. Could you take care of this deprecation, or provide more detail so a member of the STABLE team can do it?
            Hide
            Michael de Raadt added a comment -

            Eloy: I've put you in charge of this issue's parent META. Please close that one when this issue is done.

            Show
            Michael de Raadt added a comment - Eloy: I've put you in charge of this issue's parent META. Please close that one when this issue is done.
            Hide
            Michael de Raadt added a comment -

            Perhaps this issue should be put back to the next deprecation period after 2.4.

            Show
            Michael de Raadt added a comment - Perhaps this issue should be put back to the next deprecation period after 2.4.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            For the records:

            I've seen that @ moodle.org there were some filters in the list but not installed (disk) anymore. So I've deleted them from the UI too (they had a "not found" message there):

            • filter/halloween
            • filter/langflip
            • filter/medianew

            And, right now, I'm reviewing the custom filters there:

            • Geshi code highlighting
            • Moodle docs link filter
            • Moodle links filter
            • Skype icons filter

            (will create MDLSITEs if they need any action)

            Show
            Eloy Lafuente (stronk7) added a comment - For the records: I've seen that @ moodle.org there were some filters in the list but not installed (disk) anymore. So I've deleted them from the UI too (they had a "not found" message there): filter/halloween filter/langflip filter/medianew And, right now, I'm reviewing the custom filters there: Geshi code highlighting Moodle docs link filter Moodle links filter Skype icons filter (will create MDLSITEs if they need any action)
            Hide
            Sam Hemelryk added a comment - - edited

            Hi Eloy,

            Just adding a reminder there is a bug with the Geshi filter to address. If you enter the following for a forum post:

            <code>echo '<div>';</code>

            Then go back and edit it you'll get:

            <code>echo '</code><div><code>';</code></div><code></code>

            Show
            Sam Hemelryk added a comment - - edited Hi Eloy, Just adding a reminder there is a bug with the Geshi filter to address. If you enter the following for a forum post: <code>echo '<div>';</code> Then go back and edit it you'll get: <code>echo '</code><div><code>';</code></div><code></code>
            Hide
            Sam Hemelryk added a comment -

            Aha false alarm of sorts, tracked that back to a htmlpurifier bug MDL-37241

            Show
            Sam Hemelryk added a comment - Aha false alarm of sorts, tracked that back to a htmlpurifier bug MDL-37241
            Hide
            Sam Hemelryk added a comment -

            Thanks Petr
            After much staring+reading+researching I am happy with these changes and have integrated them.

            Show
            Sam Hemelryk added a comment - Thanks Petr After much staring+reading+researching I am happy with these changes and have integrated them.
            Hide
            Sam Hemelryk added a comment -

            Gah this is breaking upgrade.
            Something in the build up to upgrade is causing the filters to be inserted in with their correct names before the upgrade code executes.
            Leads to the following fatal error:

            Default exception handler: Error writing to database Debug: Duplicate entry '1-activitynames' for key 'ciu_filtacti_confil_uix'
            UPDATE ciu_filter_active SET filter = ? WHERE filter = ?
            [array (
            0 => 'activitynames',
            1 => 'filter/activitynames',
            )]
            Error code: dmlwriteexception

            • line 429 of /lib/dml/moodle_database.php: dml_write_exception thrown
            • line 1285 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end()
            • line 1565 of /lib/dml/moodle_database.php: call to mysqli_native_moodle_database->set_field_select()
            • line 1535 of /lib/db/upgrade.php: call to moodle_database->set_field()
            • line 1493 of /lib/upgradelib.php: call to xmldb_main_upgrade()
            • line 153 of /admin/cli/upgrade.php: call to upgrade_core()

            I can reproduce both through the browser and through CLI.

            Dan has mentioned something similiar has been encountered before so perhaps someone knows about it.

            Show
            Sam Hemelryk added a comment - Gah this is breaking upgrade. Something in the build up to upgrade is causing the filters to be inserted in with their correct names before the upgrade code executes. Leads to the following fatal error: Default exception handler: Error writing to database Debug: Duplicate entry '1-activitynames' for key 'ciu_filtacti_confil_uix' UPDATE ciu_filter_active SET filter = ? WHERE filter = ? [array ( 0 => 'activitynames', 1 => 'filter/activitynames', )] Error code: dmlwriteexception line 429 of /lib/dml/moodle_database.php: dml_write_exception thrown line 1285 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end() line 1565 of /lib/dml/moodle_database.php: call to mysqli_native_moodle_database->set_field_select() line 1535 of /lib/db/upgrade.php: call to moodle_database->set_field() line 1493 of /lib/upgradelib.php: call to xmldb_main_upgrade() line 153 of /admin/cli/upgrade.php: call to upgrade_core() I can reproduce both through the browser and through CLI. Dan has mentioned something similiar has been encountered before so perhaps someone knows about it.
            Hide
            Sam Hemelryk added a comment -

            Ok I've tracked the issue down to lib/pluginlib.php in which plugininfo_filter inserts plugins if they exist on the fs but not in the db.
            I've made a small commit to fix the immediate issue but someone needs to take a very close look at it still.

            Failing testing to force a look

            Show
            Sam Hemelryk added a comment - Ok I've tracked the issue down to lib/pluginlib.php in which plugininfo_filter inserts plugins if they exist on the fs but not in the db. I've made a small commit to fix the immediate issue but someone needs to take a very close look at it still. Failing testing to force a look
            Hide
            Sam Hemelryk added a comment -

            git show b8459c0

            Show
            Sam Hemelryk added a comment - git show b8459c0
            Hide
            Petr Skoda added a comment -

            oh, thanks a lot, I will retest everything and resubmit for next week

            Show
            Petr Skoda added a comment - oh, thanks a lot, I will retest everything and resubmit for next week
            Hide
            Petr Skoda added a comment -

            grrr, working on a fix now...

            Show
            Petr Skoda added a comment - grrr, working on a fix now...
            Hide
            Petr Skoda added a comment -

            the filter installation is a bloody mess - crazy!

            Show
            Petr Skoda added a comment - the filter installation is a bloody mess - crazy!
            Hide
            Petr Skoda added a comment - - edited

            one more patch pushed to my w51_MDL-29996_m25_oldfilters, sorry for not testing it properly earlier

            Show
            Petr Skoda added a comment - - edited one more patch pushed to my w51_ MDL-29996 _m25_oldfilters, sorry for not testing it properly earlier
            Hide
            Petr Skoda added a comment -

            regrrrr, I should have read you added a fix

            Show
            Petr Skoda added a comment - regrrrr, I should have read you added a fix
            Hide
            Petr Skoda added a comment -

            one more commit is HERE: https://github.com/skodak/moodle/compare/bfa2b03...filterinstall
            it is way too early here, feel free to postpone my fix if it looks weird to you

            Show
            Petr Skoda added a comment - one more commit is HERE: https://github.com/skodak/moodle/compare/bfa2b03...filterinstall it is way too early here, feel free to postpone my fix if it looks weird to you
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Not sure if that's the problem ongoing here... but I'm getting a weird error (filter/filter in paths) on upgrade with current integration.git

            Show
            Eloy Lafuente (stronk7) added a comment - Not sure if that's the problem ongoing here... but I'm getting a weird error (filter/filter in paths) on upgrade with current integration.git
            Hide
            Eloy Lafuente (stronk7) added a comment -

            I've added 8d21130267b8f922bf816086a52527c1e4c8da28 (originally fa5ec27, cherry-picked from Petr's https://github.com/skodak/moodle/compare/bfa2b03...filterinstall) and now upgrade seems to be working ok.

            Have pushed it to integration.git. Feel free to revert/destroy/whatever if it's not the correct solution.

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - I've added 8d21130267b8f922bf816086a52527c1e4c8da28 (originally fa5ec27, cherry-picked from Petr's https://github.com/skodak/moodle/compare/bfa2b03...filterinstall ) and now upgrade seems to be working ok. Have pushed it to integration.git. Feel free to revert/destroy/whatever if it's not the correct solution. Ciao
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Side comment, to avoid forgetting...

            wouldn't be better to use the component names everywhere (filter_xxxx) instead of using only the filter names? With current change we have the plugins api using the correct filter_xxx names but the filters api itself only uses xxxx (when previously it was using [filter|mod]/xxxx.

            Uhm...

            Show
            Eloy Lafuente (stronk7) added a comment - Side comment, to avoid forgetting... wouldn't be better to use the component names everywhere (filter_xxxx) instead of using only the filter names? With current change we have the plugins api using the correct filter_xxx names but the filters api itself only uses xxxx (when previously it was using [filter|mod] /xxxx. Uhm...
            Hide
            Petr Skoda added a comment -

            we could support both if you want it

            Show
            Petr Skoda added a comment - we could support both if you want it
            Hide
            Sam Hemelryk added a comment -

            Moving this back to testing - all looks like its under control now. Thanks fella's.

            Show
            Sam Hemelryk added a comment - Moving this back to testing - all looks like its under control now. Thanks fella's.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            (I'really prefer to use proper component names in all the apis, really. The mod_xxx exception is enough for me).

            Show
            Eloy Lafuente (stronk7) added a comment - (I'really prefer to use proper component names in all the apis, really. The mod_xxx exception is enough for me).
            Hide
            Michael de Raadt added a comment - - edited

            Test result: Success!

            I had some trouble with phpunit tests, but no fails or errors were related to filters or filter administration.

            I set some filters in 2.2 and upgrade to master. The state of these filters was still the same after upgrading and the filters still worked. Filters seem to be working in a fresh master site.

            Show
            Michael de Raadt added a comment - - edited Test result: Success! I had some trouble with phpunit tests, but no fails or errors were related to filters or filter administration. I set some filters in 2.2 and upgrade to master. The state of these filters was still the same after upgrading and the filters still worked. Filters seem to be working in a fresh master site.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            And your fantastic code has met core, hope they become good friends for a long period.

            Closing, thanks!

            Show
            Eloy Lafuente (stronk7) added a comment - And your fantastic code has met core, hope they become good friends for a long period. Closing, thanks!
            Hide
            Eloy Lafuente (stronk7) added a comment - - edited

            Uhm, it has been detected that while this issue was one of the very first being done after the on-sync period (only for 2.5), it used a MOODLE_24_STABLE $version to be run (2012120300.07). And that's a true problem, because people coming from, say Moodle 2.4.2 to 2.5 won't execute it.

            So we need to change that upgrade step to anything else being 2.5 only (> 20121203). Sure David comes with more details.

            Show
            Eloy Lafuente (stronk7) added a comment - - edited Uhm, it has been detected that while this issue was one of the very first being done after the on-sync period (only for 2.5), it used a MOODLE_24_STABLE $version to be run (2012120300.07). And that's a true problem, because people coming from, say Moodle 2.4.2 to 2.5 won't execute it. So we need to change that upgrade step to anything else being 2.5 only (> 20121203). Sure David comes with more details.
            Hide
            David Mudrak added a comment -

            See the linked MDL-39585 for fixing the upgrade process.

            Show
            David Mudrak added a comment - See the linked MDL-39585 for fixing the upgrade process.
            Show
            Petr Skoda added a comment - Documented at: http://docs.moodle.org/dev/Moodle_2.5_release_notes http://docs.moodle.org/dev/Filters#Two_types_of_filter

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: