monarc-project / MonarcAppFO

MONARC - Method for an Optimised aNAlysis of Risks by @NC3-LU
https://www.monarc.lu
GNU Affero General Public License v3.0
100 stars 38 forks source link

"Label required" on import risks even when label filled #484

Closed shivan closed 10 months ago

shivan commented 1 year ago

Describe the bug Exported data cannot be imported again. Even simple files cannot be imported.

To Reproduce Steps to reproduce the behavior:

  1. Go to Knowledge Base
  2. Click on Operational Risks
  3. Click on +
  4. click on "import from file"
  5. import the following file
    "code";"Label";"description";"tags"
    "test";"asdf";"test";
  6. Now you get an error "Label required"

Expected behavior Data is imported successfully

Screenshots n/a

Desktop (please complete the following information):

Questions Answers
Type of issue Bug
OS version (server) Ubuntu
OS version (client) Win10
PHP version ova image
MONARC version / git hash 2.12.6
Browser Chrome

Additional context Same occurs with xlsx import. When really leaving label empty, you get an error before import.

acherifi commented 10 months ago

I'm experciencing the same thing. The only workaround I found is to fill all the fields of the CSV file even the non mandatory ones. The error mentionned in the issue disappears but unfortunately another error happens next

Une erreur est survenue sur api/client-anr/1/rolf-risks : count(): Argument #1 ($value) must be of type Countable|array, null given (500)
acherifi commented 10 months ago

I found the issue. It seems that during the import, the code is trying to link the measures to the Operational Risk. But since the CSV does not contain any measures, the access to the measure key fails.

In order to fix this, you can modify the ApiAnrRolfRisksController.php file located here /var/lib/monarc/releases/MonarcAppFO-v2.12.7/vendor/monarc/frontoffice/src/Controller and replace the create function by the one below.

It just checks if the measures key exists before trying to get the array length. Since its empty, the import works.

I will try to do a PR later today

    public function create($data)
    {
      if (array_key_exists("measures",$data)){
          if(count($data['measures'])>0)
              $data['measures'] = $this->addAnrId($data['measures']);
      }
      return parent::create($data);
    }

This fix is incomplete since it is still needed to fill all the fields of the CSV even the non-mandatory ones.

To be clear, to use the import feature you need to:

  1. Fill all the fields of the CSV files with the following format (only the controls can be left empty)
    "code";"label";"description";"tags";"controls"
    "CODE1";"MYLABEL";"MYDESCRIPTION";"RANDOMTAG";""
  2. Replace the create function by the one above
ruslanbaidan commented 10 months ago

Initially was mentioned MONARC version 2.12.6 (as it was at the time of report), but based on the details we could see that it is 2.12.7. In any case the CSV import of Operational risks has an issue, we are going to fix it. Thank you for reporting.

ruslanbaidan commented 10 months ago

The issue is due to count function param change since php v8.0. There is a patch with a fix for this. It is going to be deployed to our production today.

acherifi commented 10 months ago

Thank you for the prompt response and reaction!

ruslanbaidan commented 10 months ago

Thank you for the report and your contribution. The patch is deployed on our production, my.monarc.lu