You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by ah...@apache.org on 2021/05/05 17:01:23 UTC

[isis] branch master updated: ISIS-2641: fixes some column visibility issues

This is an automated email from the ASF dual-hosted git repository.

ahuber pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/isis.git


The following commit(s) were added to refs/heads/master by this push:
     new 30c6706  ISIS-2641: fixes some column visibility issues
30c6706 is described below

commit 30c670607b3441503e3fe4733c45d723f3c16159
Author: Andi Huber <ah...@apache.org>
AuthorDate: Wed May 5 19:01:01 2021 +0200

    ISIS-2641: fixes some column visibility issues
---
 .../tablecol/TableColumnOrderServiceDefault.java   |   4 -
 .../dom/feature/ApplicationFeatureViewModel.java   |   8 +-
 .../wicket/model/models/EntityCollectionModel.java |  70 +++++---
 .../wicket/model/models/ManagedObjectModel.java    |   8 +-
 .../CollectionContentsAsAjaxTablePanel.java        | 178 ++++++++++-----------
 5 files changed, 142 insertions(+), 126 deletions(-)

diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/tablecol/TableColumnOrderServiceDefault.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/tablecol/TableColumnOrderServiceDefault.java
index 21d9931..f12f437 100644
--- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/tablecol/TableColumnOrderServiceDefault.java
+++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/services/tablecol/TableColumnOrderServiceDefault.java
@@ -49,7 +49,6 @@ public class TableColumnOrderServiceDefault implements TableColumnOrderService {
      * @param collectionId
      * @param collectionType
      * @param propertyIds
-     * @return
      */
     @Override
     public List<String> orderParented(
@@ -63,11 +62,8 @@ public class TableColumnOrderServiceDefault implements TableColumnOrderService {
     /**
      * Just returns the <code>propertyIds</code> unchanged.
      *
-     * @param parent
-     * @param collectionId
      * @param collectionType
      * @param propertyIds
-     * @return
      */
     @Override
     public List<String> orderStandalone(
diff --git a/extensions/security/secman/model/src/main/java/org/apache/isis/extensions/secman/model/dom/feature/ApplicationFeatureViewModel.java b/extensions/security/secman/model/src/main/java/org/apache/isis/extensions/secman/model/dom/feature/ApplicationFeatureViewModel.java
index 39e8e77..697e4f1 100644
--- a/extensions/security/secman/model/src/main/java/org/apache/isis/extensions/secman/model/dom/feature/ApplicationFeatureViewModel.java
+++ b/extensions/security/secman/model/src/main/java/org/apache/isis/extensions/secman/model/dom/feature/ApplicationFeatureViewModel.java
@@ -313,9 +313,9 @@ public abstract class ApplicationFeatureViewModel implements ViewModel {
     }
 
     // -- FACTORY
-    
+
     public static <T extends ApplicationFeatureViewModel> Function<ApplicationFeatureId, T> factory(
-            final @NonNull ApplicationFeatureRepository featureRepository, 
+            final @NonNull ApplicationFeatureRepository featureRepository,
             final @NonNull FactoryService factory,
             final @NonNull Class<T> viewmodelType) {
 
@@ -324,9 +324,9 @@ public abstract class ApplicationFeatureViewModel implements ViewModel {
     }
 
     // -- HELPER
-    
+
     protected <T extends ApplicationFeatureViewModel> List<T> asViewModels(
-            final java.util.Collection<ApplicationFeatureId> featureIds, 
+            final java.util.Collection<ApplicationFeatureId> featureIds,
             final Class<T> viewmodelType) {
         return featureIds.stream()
                 .map(factory(featureRepository, factory, viewmodelType))
diff --git a/viewers/wicket/model/src/main/java/org/apache/isis/viewer/wicket/model/models/EntityCollectionModel.java b/viewers/wicket/model/src/main/java/org/apache/isis/viewer/wicket/model/models/EntityCollectionModel.java
index e8a71ab..d634394 100644
--- a/viewers/wicket/model/src/main/java/org/apache/isis/viewer/wicket/model/models/EntityCollectionModel.java
+++ b/viewers/wicket/model/src/main/java/org/apache/isis/viewer/wicket/model/models/EntityCollectionModel.java
@@ -33,6 +33,7 @@ import javax.annotation.Nullable;
 import org.apache.wicket.Component;
 
 import org.apache.isis.applib.layout.component.CollectionLayoutData;
+import org.apache.isis.applib.services.bookmark.Bookmark;
 import org.apache.isis.commons.collections.Can;
 import org.apache.isis.commons.internal.base._NullSafe;
 import org.apache.isis.commons.internal.collections._Lists;
@@ -70,10 +71,10 @@ import lombok.val;
  * So that the model is {@link Serializable}, the {@link ManagedObject}s within
  * the collection are stored as {@link ObjectMemento}s.
  */
-public class EntityCollectionModel 
-extends ModelAbstract<List<ManagedObject>> 
-implements 
-    LinksProvider, 
+public class EntityCollectionModel
+extends ModelAbstract<List<ManagedObject>>
+implements
+    LinksProvider,
     UiHintContainer {
 
     private static final long serialVersionUID = 1L;
@@ -100,16 +101,16 @@ implements
     }
 
     public static EntityCollectionModel createStandalone(
-            ManagedObject collectionAsAdapter, 
+            ManagedObject collectionAsAdapter,
             ModelAbstract<?> model) {
 
         // dynamically determine the spec of the elements
         // (ie so a List<Object> can be rendered according to the runtime type of its elements,
         // rather than the compile-time type
         val commonSuperClassFinder = new ClassExtensions.CommonSuperclassFinder();
-        
+
         val mementoService = model.getMementoService();
-        
+
         final List<ObjectMemento> mementoList = streamElementsOf(collectionAsAdapter) // pojos
                 .filter(_NullSafe::isPresent)
                 .peek(commonSuperClassFinder::collect)
@@ -122,11 +123,11 @@ implements
                 .flatMap(specificationLoader::specForType)
                 .orElseGet(()->collectionAsAdapter.getSpecification().getElementSpecification().orElse(null));
 
-        final int pageSize = (elementSpec != null) 
+        final int pageSize = (elementSpec != null)
                 ? pageSize(elementSpec.getFacet(PagedFacet.class), PAGE_SIZE_DEFAULT_FOR_STANDALONE)
                 : PAGE_SIZE_DEFAULT_FOR_STANDALONE;
-        
-        val elementType = (elementSpec != null) 
+
+        val elementType = (elementSpec != null)
                 ? elementSpec.getCorrespondingClass()
                 : Object.class;
 
@@ -135,7 +136,7 @@ implements
                 model.getCommonContext(), Variant.STANDALONE, entityModel, elementType, pageSize);
         entityCollectionModel.mementoList = mementoList;
         return entityCollectionModel;
-        
+
     }
 
     // -- VARIANTS
@@ -165,7 +166,7 @@ implements
             @Override
             void setObject(EntityCollectionModel colModel, List<ManagedObject> adapterList) {
 
-                //XXX lombok issue, cannot use val here 
+                //XXX lombok issue, cannot use val here
                 final ObjectMementoService mementoService = colModel.getMementoService();
 
                 colModel.mementoList = _NullSafe.stream(adapterList)
@@ -223,7 +224,7 @@ implements
                 }
 
                 final List<ManagedObject> adapterList =
-                        _Lists.map(objectList, x-> (ManagedObject)colModel.getObjectManager().adapt(x));
+                        _Lists.map(objectList, x-> colModel.getObjectManager().adapt(x));
 
                 return adapterList;
             }
@@ -299,7 +300,7 @@ implements
     }
 
     @Getter private final Variant variant;
-    
+
     private final Class<?> typeOf;
     private transient Optional<ObjectSpecification> typeOfSpec;
 
@@ -343,9 +344,9 @@ implements
 
     private EntityCollectionModel(
             IsisAppCommonContext commonContext,
-            Variant type, 
-            EntityModel entityModel, 
-            Class<?> typeOf, 
+            Variant type,
+            EntityModel entityModel,
+            Class<?> typeOf,
             int pageSize) {
 
         super(commonContext);
@@ -363,10 +364,10 @@ implements
         if(collectionLayoutData == null) {
             throw new IllegalArgumentException("EntityModel must have a CollectionLayoutMetadata");
         }
-        
+
         val collectionId = collectionLayoutData.getId();
         val spec = entityModel.getTypeOfSpecification();
-        
+
         return (OneToManyAssociation) spec.getAssociationElseFail(collectionId);
     }
 
@@ -421,7 +422,7 @@ implements
     protected List<ManagedObject> load() {
         return variant.load(this);
     }
-    
+
     public @Nullable ObjectSpecification getTypeOfSpecification() {
         if (typeOfSpec == null) {
             typeOfSpec = getSpecificationLoader().specForType(typeOf);
@@ -454,7 +455,24 @@ implements
     /**
      * Populated only if {@link Variant#PARENTED}.
      */
-    @Deprecated // don't expose this implementation detail
+    public Optional<Bookmark> getParentObjectBookmark() {
+        return entityModel != null
+                ? entityModel.getManagedObject().getBookmark()
+                : Optional.empty();
+    }
+
+    /**
+     * Populated only if {@link Variant#PARENTED}.
+     */
+    public Optional<ObjectSpecification> getParentObjectSpecification() {
+        return getParentObjectBookmark()
+                .flatMap(bookmark->getCommonContext().getSpecificationLoader().specForBookmark(bookmark));
+    }
+
+    /**
+     * Populated only if {@link Variant#PARENTED}.
+     */
+    @Deprecated // don't expose this implementation detail, use getParentObjectBookmark() instead
     public ObjectMemento getParentObjectAdapterMemento() {
         return entityModel != null? entityModel.memento(): null;
     }
@@ -474,11 +492,11 @@ implements
     public boolean toggleSelectionOn(ManagedObject selectedAdapter) {
         //XXX lombok issue, cannot use val here
         final ObjectMemento selectedAsMemento = super.getMementoService().mementoForObject(selectedAdapter);
-        final String selectedKey = selectedAsMemento.asString(); 
-        
-        final boolean isSelected = _Maps.toggleElement(toggledMementos, selectedKey, selectedAsMemento); 
+        final String selectedKey = selectedAsMemento.asString();
+
+        final boolean isSelected = _Maps.toggleElement(toggledMementos, selectedKey, selectedAsMemento);
         return isSelected;
-        
+
     }
 
     public Can<ObjectMemento> getToggleMementosList() {
@@ -544,4 +562,6 @@ implements
     }
 
 
+
+
 }
diff --git a/viewers/wicket/model/src/main/java/org/apache/isis/viewer/wicket/model/models/ManagedObjectModel.java b/viewers/wicket/model/src/main/java/org/apache/isis/viewer/wicket/model/models/ManagedObjectModel.java
index 897ca57..d1353ee 100644
--- a/viewers/wicket/model/src/main/java/org/apache/isis/viewer/wicket/model/models/ManagedObjectModel.java
+++ b/viewers/wicket/model/src/main/java/org/apache/isis/viewer/wicket/model/models/ManagedObjectModel.java
@@ -135,7 +135,7 @@ extends ModelAbstract<ManagedObject> {
                 .map(ObjectMemento::getLogicalType);
     }
     
-    private transient ObjectSpecification objectSpec;
+    private transient ObjectSpecification elementTypeSpec;
     private transient boolean isObjectSpecMemoized = false;
     /**
      * @implNote can be overridden by sub-models (eg {@link ScalarModel}) that know the type of
@@ -146,10 +146,10 @@ extends ModelAbstract<ManagedObject> {
     public ObjectSpecification getTypeOfSpecification() {
         if(!isObjectSpecMemoized) {
             val logicalType = getLogicalElementType().orElse(null);
-            objectSpec = super.getSpecificationLoader().specForLogicalType(logicalType).orElse(null);
+            elementTypeSpec = super.getSpecificationLoader().specForLogicalType(logicalType).orElse(null);
             isObjectSpecMemoized = true;
         }
-        return objectSpec;
+        return elementTypeSpec;
     }
 
     
@@ -204,7 +204,7 @@ extends ModelAbstract<ManagedObject> {
         val manageObject = super.getCommonContext().reconstructObject(memento);
         super.setObject(manageObject);
         this.memento = memento;
-        this.objectSpec = null; // invalidate
+        this.elementTypeSpec = null; // invalidate
     }
 
 
diff --git a/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/collectioncontents/ajaxtable/CollectionContentsAsAjaxTablePanel.java b/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/collectioncontents/ajaxtable/CollectionContentsAsAjaxTablePanel.java
index 3a435cf..67dd8ee 100644
--- a/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/collectioncontents/ajaxtable/CollectionContentsAsAjaxTablePanel.java
+++ b/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/collectioncontents/ajaxtable/CollectionContentsAsAjaxTablePanel.java
@@ -23,8 +23,10 @@ import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.function.Predicate;
-import java.util.stream.Stream;
+
+import javax.annotation.Nullable;
 
 import org.apache.wicket.Component;
 import org.apache.wicket.extensions.ajax.markup.html.repeater.data.table.AjaxFallbackDefaultDataTable;
@@ -34,10 +36,10 @@ import org.apache.wicket.model.Model;
 import org.apache.isis.applib.annotation.Where;
 import org.apache.isis.applib.services.tablecol.TableColumnOrderService;
 import org.apache.isis.applib.services.tablecol.TableColumnVisibilityService;
-import org.apache.isis.commons.collections.Can;
+import org.apache.isis.commons.internal.base._NullSafe;
 import org.apache.isis.commons.internal.collections._Lists;
 import org.apache.isis.commons.internal.collections._Maps;
-import org.apache.isis.core.metamodel.facetapi.Facet;
+import org.apache.isis.commons.internal.functions._Predicates;
 import org.apache.isis.core.metamodel.facets.WhereValueFacet;
 import org.apache.isis.core.metamodel.facets.all.describedas.DescribedAsFacet;
 import org.apache.isis.core.metamodel.facets.all.hide.HiddenFacet;
@@ -51,7 +53,6 @@ import org.apache.isis.core.runtime.memento.ObjectMemento;
 import org.apache.isis.viewer.wicket.model.models.EntityCollectionModel;
 import org.apache.isis.viewer.wicket.ui.components.collection.bulk.BulkActionsProvider;
 import org.apache.isis.viewer.wicket.ui.components.collection.count.CollectionCountProvider;
-import org.apache.isis.viewer.wicket.ui.components.collectioncontents.ajaxtable.columns.ColumnAbstract;
 import org.apache.isis.viewer.wicket.ui.components.collectioncontents.ajaxtable.columns.ObjectAdapterPropertyColumn;
 import org.apache.isis.viewer.wicket.ui.components.collectioncontents.ajaxtable.columns.ObjectAdapterTitleColumn;
 import org.apache.isis.viewer.wicket.ui.components.collectioncontents.ajaxtable.columns.ObjectAdapterToggleboxColumn;
@@ -142,12 +143,17 @@ implements CollectionCountProvider {
     }
 
     private void addPropertyColumnsIfRequired(final List<IColumn<ManagedObject, String>> columns) {
-        final ObjectSpecification typeOfSpec = getModel().getTypeOfSpecification();
+        val collectionModel = getModel();
+
+        val typeOfSpec = collectionModel.getTypeOfSpecification();
         final Comparator<String> propertyIdComparator;
 
         if(typeOfSpec == null) {
             return;
         }
+
+        val elementType = typeOfSpec.getCorrespondingClass();
+
         // same code also appears in EntityPage.
         // we need to do this here otherwise any tables will render the columns in the wrong order until at least
         // one object of that type has been rendered via EntityPage.
@@ -177,113 +183,107 @@ implements CollectionCountProvider {
             propertyIdComparator = null;
         }
 
-        final Where whereContext =
-                getModel().isParented()
+        val whereContext = collectionModel.isParented()
                     ? Where.PARENTED_TABLES
                     : Where.STANDALONE_TABLES;
 
-        final ObjectSpecification parentSpecIfAny =
-                getModel().isParented()
-                    ? getCommonContext().reconstructObject(getModel().getParentObjectAdapterMemento())
-                            .getSpecification()
-                        : null;
-
-        final Class<?> collectionType = getModel().getTypeOfSpecification().getCorrespondingClass();
-
-        final Predicate<ObjectAssociation> predicate = ObjectAssociation.Predicates.PROPERTIES
-                .and((final ObjectAssociation association)->{
-                    final Stream<Facet> facets = association.streamFacets()
-                            .filter((final Facet facet)->
-                                    facet instanceof HiddenFacet);
-                    return facets
-                            .map(facet->(WhereValueFacet) facet)
-                            .noneMatch(wawF->wawF.where().includes(whereContext));
-
-                })
-                .and(associationDoesNotReferenceParent(parentSpecIfAny))
-                .and(objectAssociation -> {
-                    // optional SPI to filter
-                    final Can<TableColumnVisibilityService> tableColumnVisibilityServices =
-                            getServiceRegistry().select(TableColumnVisibilityService.class);
-
-                    final boolean atLeastOneHidden =
-                            tableColumnVisibilityServices.stream()
-                            .anyMatch(x -> x.hides(collectionType, objectAssociation.getId()));
-                    return !atLeastOneHidden;
-                });
-
-        final Stream<? extends ObjectAssociation> propertyList =
-                typeOfSpec.streamAssociations(MixedIn.INCLUDED)
-                .filter(predicate)
-                ;
-
-        final Map<String, ObjectAssociation> propertyById = _Maps.newLinkedHashMap();
-        propertyList.forEach(property->
-        propertyById.put(property.getId(), property));
-
-        List<String> propertyIds = _Lists.newArrayList(propertyById.keySet());
+        val parentSpecIfAny =  collectionModel.getParentObjectSpecification()
+                .orElse(null);
+
+        val propertyById = _Maps.<String, ObjectAssociation>newLinkedHashMap();
+
+        typeOfSpec.streamAssociations(MixedIn.INCLUDED)
+        .filter(ObjectAssociation.Predicates.PROPERTIES)
+        .filter(property->property.streamFacets()
+                    .filter(facet -> facet instanceof WhereValueFacet)
+                    .map(WhereValueFacet.class::cast)
+                    .map(WhereValueFacet::where)
+                    .noneMatch(where -> !where.includes(whereContext)))
+        .filter(associationDoesNotReferenceParent(parentSpecIfAny))
+        //FIXME is broken
+        //.filter(property->filterColumnsUsingSpi(property, elementType)) // optional SPI to filter
+        .forEach(property->propertyById.put(property.getId(), property));
+
+        val propertyIdsInOrder = _Lists.<String>newArrayList(propertyById.keySet());
 
         if(propertyIdComparator!=null) {
-            propertyIds.sort(propertyIdComparator);
+            propertyIdsInOrder.sort(propertyIdComparator);
         }
 
-        // optional SPI to reorder
-        final Can<TableColumnOrderService> tableColumnOrderServices =
-                getServiceRegistry().select(TableColumnOrderService.class);
+        // optional SPI to reorder columns
+        sortColumnsUsingSpi(propertyIdsInOrder, elementType);
 
-        for (final TableColumnOrderService tableColumnOrderService : tableColumnOrderServices) {
-            final List<String> propertyReorderedIds = reordered(tableColumnOrderService, collectionType, propertyIds);
-            if(propertyReorderedIds != null) {
-                propertyIds = propertyReorderedIds;
-                break;
-            }
-        }
+        // add all ordered columns to the table
+        propertyIdsInOrder.stream()
+        .map(propertyById::get)
+        .filter(_NullSafe::isPresent)
+        .map(this::createObjectAdapterPropertyColumn)
+        .forEach(columns::add);
 
-        for (final String propertyId : propertyIds) {
-            final ObjectAssociation property = propertyById.get(propertyId);
-            if(property != null) {
-                final ColumnAbstract<ManagedObject> nopc = createObjectAdapterPropertyColumn(property);
-                columns.add(nopc);
-            }
-        }
     }
 
-    private List<String> reordered(
-            final TableColumnOrderService tableColumnOrderService,
-            final Class<?> collectionType,
-            final List<String> propertyIds) {
+    private boolean filterColumnsUsingSpi(
+            final ObjectAssociation property,
+            final Class<?> elementType) {
+        return getServiceRegistry()
+                .select(TableColumnVisibilityService.class)
+                .stream()
+                .noneMatch(x -> x.hides(elementType, property.getId()));
+    }
 
-        final ObjectMemento parentObjectAdapterMemento = getModel().getParentObjectAdapterMemento();
-        if(parentObjectAdapterMemento != null) {
-            val parentObjectAdapter = getCommonContext().reconstructObject(parentObjectAdapterMemento);
-            final Object parent = parentObjectAdapter.getPojo();
-            final String collectionId = getModel().getCollectionMemento().getId();
+    private void sortColumnsUsingSpi(
+            final List<String> propertyIdsInOrder,
+            final Class<?> elementType) {
 
-            return tableColumnOrderService.orderParented(parent, collectionId, collectionType, propertyIds);
-        } else {
-            return tableColumnOrderService.orderStandalone(collectionType, propertyIds);
+        val tableColumnOrderServices = getServiceRegistry().select(TableColumnOrderService.class);
+        if(tableColumnOrderServices.isEmpty()) {
+            return;
         }
+
+        val parentObject = Optional.ofNullable(getModel().getParentObjectAdapterMemento())
+                .map(getCommonContext()::reconstructObject);
+
+        tableColumnOrderServices.stream()
+        .map(tableColumnOrderService->
+            parentObject.isPresent()
+                ? tableColumnOrderService.orderParented(
+                        parentObject.get().getPojo(),
+                        getModel().getCollectionMemento().getId(),
+                        elementType,
+                        propertyIdsInOrder)
+
+                : tableColumnOrderService.orderStandalone(
+                        elementType,
+                        propertyIdsInOrder)
+
+                )
+        .filter(_NullSafe::isPresent)
+        .findFirst()
+        .filter(propertyReorderedIds->propertyReorderedIds!=propertyIdsInOrder) // skip if its the same object
+        .ifPresent(propertyReorderedIds->{
+            propertyIdsInOrder.clear();
+            propertyIdsInOrder.addAll(propertyReorderedIds);
+        });
+
     }
 
-    static Predicate<ObjectAssociation> associationDoesNotReferenceParent(final ObjectSpecification parentSpec) {
+    static Predicate<ObjectAssociation> associationDoesNotReferenceParent(
+            final @Nullable ObjectSpecification parentSpec) {
         if(parentSpec == null) {
-            return __->true;
+            return _Predicates.alwaysTrue();
         }
-        return new Predicate<ObjectAssociation>() {
-            @Override
-            public boolean test(ObjectAssociation association) {
-                final HiddenFacet facet = association.getFacet(HiddenFacet.class);
-                if(facet == null) {
+        return (ObjectAssociation property) -> {
+                val hiddenFacet = property.getFacet(HiddenFacet.class);
+                if(hiddenFacet == null) {
                     return true;
                 }
-                if (facet.where() != Where.REFERENCES_PARENT) {
+                if (hiddenFacet.where() != Where.REFERENCES_PARENT) {
                     return true;
                 }
-                final ObjectSpecification assocSpec = association.getSpecification();
-                final boolean associationSpecIsOfParentSpec = parentSpec.isOfType(assocSpec);
-                final boolean isVisible = !associationSpecIsOfParentSpec;
+                val propertySpec = property.getSpecification();
+                final boolean propertySpecIsOfParentSpec = parentSpec.isOfType(propertySpec);
+                final boolean isVisible = !propertySpecIsOfParentSpec;
                 return isVisible;
-            }
         };
     }