mahara-contributors team mailing list archive

[Rebecca Blundell <1897981@xxxxxxxxxxxxxxxxxx>]

After further investigation I've found there are a number of issues at
play here. My initial assumption that the import wasn't importing a
particular file is wrong, but I'll lay out the issues as I understand
them because I think there are improvements to be made:

Testing on the QA site:
I tried to import a zip file containing a leap2a export.
I saw an nginx error 413, file too large.
I extracted the zip and tried to import the leap2a.xml thinking it would look in the files folder for the files.
It looked like it worked because all the info from the xml was loaded. There is no error saying you must import a zip. On the page it doesn't say you should import a zip.
When I pressed import, the error message showed up.
There should be an exception displayed at this point, probably for all of the files that didn't import. What happens is you see an error message which is thrown by the exception that the artefact can't be found. See the following:

        if (is_null($artefacts) || count($artefacts) != 1) {
            // This can happen if a Leap2A xml file is uploaded that refers to
            // files that (naturally) weren't uploaded with it.
            log_debug("Warning: fixref was expecting one artefact to have been imported by entry {$hrefsrc} but seems to have gotten " . count($artefacts));
            return $hrefsrc;
        }

The file doesn't exist and $artefacts is null. The check for is_null
triggers the log_debug to run, but the log_debug has a call to count(),
which will generate a php error as you can't call count on null. We
should adapt this message to avoid calling count() on null.


Testing on my local environment:

There are two potential traps here:
1. For some reason, I didn't have php7.x-zip installed, which meant I thought I was supposed to extract the zip before trying to upload. Having rectified that, I was able to upload the file without errors.
2. If you failed to change your php.ini settings to allow file upload above a certain size, you won't be able to upload the zip. 

These two things would be out of the hands of a regular user. It would
be nice to see an error message about php-zip not being installed
though.


Summary:

Things to fix:
The exception above which is thrown when an artefact is not found should not generate an error message.

Feature requests related to this (need review):
1. Message on the import page that says you should be uploading a zip archive
2. If php-zip is looked for and not found, an error message would be nice.
3. QA Mahara seems to have a very low file upload limit set in Nginx. It would be good to bump this to at least 32MB in line with what we use typically in dev set-up for apache2.

-- 
You received this bug notification because you are a member of Mahara
Contributors, which is subscribed to Mahara.
Matching subscriptions: Subscription for all Mahara Contributors -- please ask on #mahara-dev or mahara.org forum before editing or unsubscribing it!
https://bugs.launchpad.net/bugs/1897981

Title:
  Leap2A import shows Countable error

Status in Mahara:
  Confirmed
Status in Mahara 20.10 series:
  Confirmed
Status in Mahara 21.04 series:
  In Progress

Bug description:
  Using QA Mahara with the following settings on Windows:
  PHP 7.4
  Postgres 10.9
  Skins enabled
  Individual themes enabled
  Pre-populated data
  Maroon theme
  Chrome

  Steps taken:
  1. Log in as a regular person
  2. Import a leap2a file

  Expected:
  You see:
  Your portfolio was imported successfully

  Actual
  The portfolio imports, but there is the following error, which was a PHP 7.2 change to count()(see screenshot):

  
  [WAR] f7 (import/leap/lib.php:1774) count(): Parameter must be an array or an object that implements Countable
  Call stack (most recent first):
  log_message("count(): Parameter must be an array or an object t...", 8, true, true, "/var/www/mahara/htdocs/import/leap/lib.php", 1774) at /var/www/mahara/htdocs/lib/errors.php:521
  error(2, "count(): Parameter must be an array or an object t...", "/var/www/mahara/htdocs/import/leap/lib.php", 1774, array(size 3)) at /var/www/mahara/htdocs/import/leap/lib.php:1774
  PluginImportLeap->_fixref("portfolio:artefact110") at /var/www/mahara/htdocs/import/leap/lib.php:1721
  PluginImportLeap->fix_artefact_reference("<div> <p><img width="640" alt="md_5ac44dbd9de98.pn...") at /var/www/mahara/htdocs/import/leap/lib.php:1663
  PluginImportLeap->fix_artefact_references(object(stdClass)) at /var/www/mahara/htdocs/import/leap/lib.php:344
  PluginImportLeap->do_import_from_requests() at /var/www/mahara/htdocs/import/index.php:314
  do_import() at /var/www/mahara/htdocs/import/index.php:71
  [DBG] f7 (import/leap/lib.php:1774) Warning: fixref was expecting one artefact to have been imported by entry portfolio:artefact110 but seems to have gotten 0

  
  This needs some investigation to see what the $artefact variable contains at the time count is called on it:
   $artefacts = $this->get_artefactids_imported_by_entryid($hrefsrc);
          if (is_null($artefacts) || count($artefacts) != 1) {
              // This can happen if a Leap2A xml file is uploaded that refers to
              // files that (naturally) weren't uploaded with it.
              log_debug("Warning: fixref was expecting one artefact to have been imported by entry {$hrefsrc} but seems to have gotten " . count($artefacts));
              return $hrefsrc;

  It looks like it probably contains a number, which won't work with PHP
  7.2 and later, it needs to be an array or object from memory. For more
  info see: https://www.php.net/manual/en/function.count.php

  Note: have not tested on 20.04 to see if this is a regression.

To manage notifications about this bug go to:
https://bugs.launchpad.net/mahara/+bug/1897981/+subscriptions