Edit

Andreas Müller wrote:

This works now for taxon nodes not sharing duplicates (e.g. for sibblings). It still shows duplicates if e.g. one selects a parent and some of it's children. The later should either not be possible (I don't know if this is easy to implement in the navigator) or it should be handled such that the algorithm should recognize the child duplicates and remove them.

Can you please review my changes and either implement the open issue or create a new ticket?

Your changes work fine. To forbid the selection of a parent and some of his children is a little bit more difficult, I adapt the algorithm to avoid duplicates of children as a first step, but maybe this is a little bit confusing because the user selected the parent and for example 4 of 6 children but gets all 6 children.

Andreas Müller wrote:

Katja Luther wrote:

please review my changes.

Have you tested this? It does not work. And it can't work as parent seem to be handled elsewhere. So the selected children are also added as parents (because they are selected).
This maybe happens on TaxEditor side or elsewhere but for sure not in getTaxonDistributionDTO()

Yes, I tested it locally. I will test again with nightly.

Katja Luther wrote:

Andreas Müller wrote:

Katja Luther wrote:

please review my changes.

Have you tested this? It does not work. And it can't work as parent seem to be handled elsewhere. So the selected children are also added as parents (because they are selected).
This maybe happens on TaxEditor side or elsewhere but for sure not in getTaxonDistributionDTO()

Yes, I tested it locally. I will test again with nightly.

Upps, maybe only a server restart is needed as this is cdmlib-side. However, I do not understand the handling of parents in the method. How are they added to the final result?

Andreas Müller wrote:

Katja Luther wrote:

Andreas Müller wrote:

Katja Luther wrote:

please review my changes.

Have you tested this? It does not work. And it can't work as parent seem to be handled elsewhere. So the selected children are also added as parents (because they are selected).
This maybe happens on TaxEditor side or elsewhere but for sure not in getTaxonDistributionDTO()

Yes, I tested it locally. I will test again with nightly.

Upps, maybe only a server restart is needed as this is cdmlib-side. However, I do not understand the handling of parents in the method. How are they added to the final result?

The criteria for getting the children is

        crit.add( Restrictions.like("treeIndex", node.treeIndex()+ "%") );

therefore also the node itself is found.

Katja Luther wrote:

The criteria for getting the children is

        crit.add( Restrictions.like("treeIndex", node.treeIndex()+ "%") );

therefore also the node itself is found.

Yes, but later there is

results.remove(node);

which removes the parent node. The correct behavior is tested in TaxonNodeDaoHibernateImplTest.testListChildren(). If the result contains the parent, too, we urgently need to fix the method or rename it because this is not the expected behavior for the given method name I guess.

Andreas Müller wrote:

Katja Luther wrote:

The criteria for getting the children is

        crit.add( Restrictions.like("treeIndex", node.treeIndex()+ "%") );

therefore also the node itself is found.

Yes, but later there is

results.remove(node);

which removes the parent node. The correct behavior is tested in TaxonNodeDaoHibernateImplTest.testListChildren(). If the result contains the parent, too, we urgently need to fix the method or rename it because this is not the expected behavior for the given method name I guess.

Sorry it is the method listChildrenOfRecursive, there the node is added to the result:

if (!previousResult.contains(node)){
            previousResult.add(node);
        }

Katja Luther wrote:

Sorry it is the method listChildrenOfRecursive, there the node is added to the result:

if (!previousResult.contains(node)){
            previousResult.add(node);
        }

Ok, this is obviously an error in the method with recursive=true and comparator != null as the method returns another collection size then calling it without comparator. I fixed this and added a test.
As the parent is now missing I added it in getTaxonDistributionDTO() instead which IMO is the correct place for adding.

Can you please review? I havn't tested the result yet.