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:
    • Rank:
      19540

      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

        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 Škoda added a comment -

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

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

          grrr, working on a fix now...

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

          the filter installation is a bloody mess - crazy!

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

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

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

          regrrrr, I should have read you added a fix

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

          we could support both if you want it

          Show
          Petr Škoda 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 Škoda 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: