From 83635b049a37229100f0307bcf04783d689b9101 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=B4me=20Chilliet?= <come@opensides.be>
Date: Tue, 19 Mar 2019 11:46:31 +0100
Subject: [PATCH] :ambulance: fix(management) Fix attributes values handling

Column attributes are now always considered multivalued and returned
 value is always an array of values, which may be empty.
Code has been adapted to support this.
This means plugins will not need to specify '*' anymore for
 multivaluated attributes.

issue #5973
---
 include/exporter/class_csvExporter.inc        |  2 +-
 include/exporter/class_pdfExporter.inc        | 18 +++-
 include/management/columns/class_Column.inc   | 85 +++++++++----------
 .../class_LdapGeneralizedTimeColumn.inc       |  3 +-
 .../columns/class_UnixTimestampColumn.inc     |  4 +-
 .../admin/groups/class_GroupContentColumn.inc |  5 +-
 6 files changed, 63 insertions(+), 54 deletions(-)

diff --git a/include/exporter/class_csvExporter.inc b/include/exporter/class_csvExporter.inc
index e45d70e98..c26cc9ef5 100644
--- a/include/exporter/class_csvExporter.inc
+++ b/include/exporter/class_csvExporter.inc
@@ -101,7 +101,7 @@ class csvExporter
     foreach ($iterator as $entry) {
       foreach ($columns as $column) {
         if ($column->isExportable()) {
-          $result .= $column->getRawExportValue($entry).';';
+          $result .= implode(',', $column->getRawExportValues($entry)).';';
         }
       }
       $result = preg_replace('/;$/', '', $result)."\n";
diff --git a/include/exporter/class_pdfExporter.inc b/include/exporter/class_pdfExporter.inc
index 69f77d357..1fbc99188 100644
--- a/include/exporter/class_pdfExporter.inc
+++ b/include/exporter/class_pdfExporter.inc
@@ -196,7 +196,7 @@ class pdfExporter
       }
 
       foreach ($iterator as $entry) {
-        $len = $result->GetStringWidth($column->getRawExportValue($entry));
+        $len = $result->GetStringWidth(implode(',', $column->getRawExportValues($entry)));
         if ($len > $max) {
           $max = $len;
         }
@@ -282,7 +282,21 @@ class pdfExporter
 
       foreach ($columns as $index => $column) {
         if ($column->isExportable()) {
-          $result->Cell($width[$index], 6, utf8_decode($column->getRawExportValue($entry)), 'LR', 0, 'L', $fill);
+          $result->Cell(
+            $width[$index],
+            6,
+            implode(
+              ',',
+              array_map(
+                'utf8_decode',
+                $column->getRawExportValues($entry)
+              )
+            ),
+            'LR',
+            0,
+            'L',
+            $fill
+          );
         }
       }
 
diff --git a/include/management/columns/class_Column.inc b/include/management/columns/class_Column.inc
index 77f29ec81..4c717755a 100644
--- a/include/management/columns/class_Column.inc
+++ b/include/management/columns/class_Column.inc
@@ -25,11 +25,10 @@ class Column
 {
   /*! \brief Array of attributes to look for, ordered by priority
    * The first non-empty attribute will be displayed
-   * The array is organized as the one passed to objects::ls
    * */
-  private $attributes;
+  protected $attributes;
   /*! \brief Same thing for templates, if it differs */
-  private $templateAttributes = NULL;
+  protected $templateAttributes = NULL;
   protected $label;
   protected $type = 'string';
 
@@ -69,19 +68,7 @@ class Column
 
   protected function setAttributesVar (string $var, array $attributes = NULL)
   {
-    if (is_array($attributes) && is_numeric(key($attributes))) {
-      $val = [];
-      foreach ($attributes as $attribute) {
-        if ($attribute == 'dn') {
-          $val[$attribute] = 'raw';
-        } else {
-          $val[$attribute] = 1;
-        }
-      }
-      $this->$var = $val;
-    } else {
-      $this->$var = $attributes;
-    }
+    $this->$var = $attributes;
   }
 
   function setTemplateAttributes (array $attributes = NULL)
@@ -121,12 +108,17 @@ class Column
   function fillNeededAttributes (array &$attrs)
   {
     if (isset($this->attributes)) {
-      foreach ($this->attributes as $attr => $how) {
+      foreach ($this->attributes as $attr) {
         if (($attr == 'mainAttr') || ($attr == 'nameAttr')) {
           /* nameAttr and mainAttr as always set as needed in managementFilter */
           continue;
+        } elseif ($attr == 'dn') {
+          /* Handle special case of dn */
+          $attrs[$attr] = 'raw';
+        } else {
+          /* Get all values from other attributes */
+          $attrs[$attr] = '*';
         }
-        $attrs[$attr] = $how;
       }
     }
   }
@@ -134,7 +126,7 @@ class Column
   function fillSearchedAttributes (array &$attrs)
   {
     if (isset($this->attributes)) {
-      foreach (array_keys($this->attributes) as $attr) {
+      foreach ($this->attributes as $attr) {
         if (($attr == 'mainAttr') || ($attr == 'nameAttr')) {
           /* nameAttr and mainAttr as always searched for */
           continue;
@@ -150,60 +142,63 @@ class Column
   {
   }
 
-  protected function getAttributeValue (ListingEntry $entry)
+  protected function getAttributeValues (ListingEntry $entry): array
   {
     $attrs = $this->attributes;
     if (isset($this->templateAttributes) && $entry->isTemplate()) {
       $attrs = $this->templateAttributes;
     }
     if (isset($attrs)) {
-      foreach (array_keys($attrs) as $attr) {
+      foreach ($attrs as $attr) {
         if (($attr == 'mainAttr') || ($attr == 'nameAttr')) {
           $infos  = objects::infos($entry->getTemplatedType());
           $attr   = $infos[$attr];
         }
         if (isset($entry[$attr])) {
-          return $entry[$attr];
+          if (is_array($entry[$attr])) {
+            return $entry[$attr];
+          } else {
+            /* Should only happen for dn */
+            return [$entry[$attr]];
+          }
         }
       }
     }
-    return '';
+    return [];
   }
 
   function renderCell (ListingEntry $entry): string
   {
-    $value = $this->getAttributeValue($entry);
-    if ($value == '') {
+    $values = $this->getAttributeValues($entry);
+    if (empty($values)) {
       return '&nbsp;';
-    } elseif (is_array($value)) {
-      $value = array_map(
-        function ($v)
-        {
-          return htmlentities($v, ENT_COMPAT, 'UTF-8');
-        },
-        $value
-      );
-      return implode("<br/>\n", $value);
     } else {
-      return htmlentities($value, ENT_COMPAT, 'UTF-8');
+      return implode("<br/>\n",
+        array_map(
+          function ($value) use ($entry)
+          {
+            return $this->renderSingleValue($entry, $value);
+          },
+          $values
+        )
+      );
     }
   }
 
-  function getRawExportValue (ListingEntry $entry)
+  protected function renderSingleValue (ListingEntry $entry, string $value): string
   {
-    return $this->getAttributeValue($entry);
+    return htmlentities($value, ENT_COMPAT, 'UTF-8');
+  }
+
+  function getRawExportValues (ListingEntry $entry): array
+  {
+    return $this->getAttributeValues($entry);
   }
 
   function compare (ListingEntry $ao, ListingEntry $bo): int
   {
-    $a = $this->getAttributeValue($ao);
-    $b = $this->getAttributeValue($bo);
-    if (is_array($a)) {
-      $a = $a[0];
-    }
-    if (is_array($b)) {
-      $b = $b[0];
-    }
+    $a = $this->getAttributeValues($ao)[0] ?? '';
+    $b = $this->getAttributeValues($bo)[0] ?? '';
 
     // Take a look at the several types
     switch ($this->type) {
diff --git a/include/management/columns/class_LdapGeneralizedTimeColumn.inc b/include/management/columns/class_LdapGeneralizedTimeColumn.inc
index d01df1ce1..b64a49b0c 100644
--- a/include/management/columns/class_LdapGeneralizedTimeColumn.inc
+++ b/include/management/columns/class_LdapGeneralizedTimeColumn.inc
@@ -26,9 +26,8 @@ class LdapGeneralizedTimeColumn extends LinkColumn
 {
   protected $type = 'string';
 
-  function renderCell (ListingEntry $entry): string
+  protected function renderSingleValue (ListingEntry $entry, string $value): string
   {
-    $value = $this->getAttributeValue($entry);
     if ($value != '') {
       $dateObject = LdapGeneralizedTime::fromString($value);
       if (is_object($dateObject)) {
diff --git a/include/management/columns/class_UnixTimestampColumn.inc b/include/management/columns/class_UnixTimestampColumn.inc
index 1861e46ac..601380f1f 100644
--- a/include/management/columns/class_UnixTimestampColumn.inc
+++ b/include/management/columns/class_UnixTimestampColumn.inc
@@ -1,6 +1,7 @@
 <?php
 /*
   This code is part of FusionDirectory (http://www.fusiondirectory.org/)
+
   Copyright (C) 2018-2019  FusionDirectory
 
   This program is free software; you can redistribute it and/or modify
@@ -25,9 +26,8 @@ class UnixTimestampColumn extends LinkColumn
 {
   protected $type = 'integer';
 
-  function renderCell (ListingEntry $entry): string
+  protected function renderSingleValue (ListingEntry $entry, string $value): string
   {
-    $value = $this->getAttributeValue($entry);
     if ($value != '') {
       $dateObject = DateTime::createFromFormat('U', $value, new DateTimeZone('UTC'));
       if (is_object($dateObject)) {
diff --git a/plugins/admin/groups/class_GroupContentColumn.inc b/plugins/admin/groups/class_GroupContentColumn.inc
index 4a848237a..7b6f2db6f 100644
--- a/plugins/admin/groups/class_GroupContentColumn.inc
+++ b/plugins/admin/groups/class_GroupContentColumn.inc
@@ -1,7 +1,8 @@
 <?php
 /*
   This code is part of FusionDirectory (http://www.fusiondirectory.org/)
-  Copyright (C) 2017-2018  FusionDirectory
+
+  Copyright (C) 2017-2019  FusionDirectory
 
   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
@@ -28,7 +29,7 @@ class GroupContentColumn extends Column
     global $config;
 
     if (strtolower($entry->getTemplatedType()) == 'ogroup') {
-      $types = preg_replace('/[^a-z]/i', '', $this->getAttributeValue($entry));
+      $types = preg_replace('/[^a-z]/i', '', implode('', $this->getAttributeValues($entry)));
     } else {
       $types = 'U';
     }
-- 
GitLab