Details

    • Type: Sub-task
    • Status: Closed
    • Priority: 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

          Attachments

            Issue Links

              Activity

              Hide
              stronk7 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
              stronk7 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
              salvetore 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
              salvetore 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
              salvetore 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
              salvetore 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
              salvetore Michael de Raadt added a comment -

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

              Show
              salvetore Michael de Raadt added a comment - Perhaps this issue should be put back to the next deprecation period after 2.4.
              Hide
              stronk7 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
              stronk7 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
              samhemelryk 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
              samhemelryk 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
              samhemelryk Sam Hemelryk added a comment -

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

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

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

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks Petr After much staring+reading+researching I am happy with these changes and have integrated them.
              Hide
              samhemelryk 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
              samhemelryk 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
              samhemelryk 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
              samhemelryk 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
              samhemelryk Sam Hemelryk added a comment -

              git show b8459c0

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

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

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

              grrr, working on a fix now...

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

              the filter installation is a bloody mess - crazy!

              Show
              skodak Petr Skoda added a comment - the filter installation is a bloody mess - crazy!
              Hide
              skodak 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
              skodak 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
              skodak Petr Skoda added a comment -

              regrrrr, I should have read you added a fix

              Show
              skodak Petr Skoda added a comment - regrrrr, I should have read you added a fix
              Hide
              skodak 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
              skodak 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
              stronk7 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
              stronk7 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
              stronk7 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
              stronk7 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
              stronk7 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
              stronk7 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
              skodak Petr Skoda added a comment -

              we could support both if you want it

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

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

              Show
              samhemelryk Sam Hemelryk added a comment - Moving this back to testing - all looks like its under control now. Thanks fella's.
              Hide
              stronk7 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
              stronk7 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
              salvetore 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
              salvetore 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
              stronk7 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
              stronk7 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
              stronk7 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
              stronk7 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
              mudrd8mz David Mudrák added a comment -

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

              Show
              mudrd8mz David Mudrák added a comment - See the linked MDL-39585 for fixing the upgrade process.
              Show
              skodak 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:
                    Fix Release Date:
                    14/May/13