NextrasDataSource faulty checks

[mskocik]

Although NextrasDatasource accepts ICollection in constructor, comparisons for Nextras\Orm v4 added in #920 and #937 take into account only DbalCollection.

One would expect ICollection implementations usable.

[radimvaculik]

The best option is drop support for v3.1, leave v4 only and refactor it to ICollection everywhere. Current state is not perfect at all, but it's good compromise between backward compatibility with v3.1 and v4.

[f3l1x]

I am in. Let's support only v4.

[mskocik]

My point wasn't about v3/v4 difference, but inpropriate collection type enforcement.

This can happen when you use Nextras\Orm\Repository->findByIds(). Although DbalCollection is usually returned, there are cases when ArrayCollection is returned instead.

datagrid/src/DataSource/NextrasDataSource.php

Lines 130 to 138 in 1b0ff6a

	if (!$this->dataSource instanceof $this->dbalCollectionClass) {
	throw new UnexpectedValueException(
	sprintf(
	'Expeting %s, got %s',
	$this->dbalCollectionClass,
	get_class($this->dataSource)
	)
	);
	}

datagrid/src/DataSource/NextrasDataSource.php

Lines 143 to 149 in 1b0ff6a

	$order = $this->dataSource->getQueryBuilder()->getClause('order');
	if (ArraysHelper::testEmpty($order)) {
	$this->dataSource = $this->dataSource->orderBy(
	$this->prepareColumn($this->primaryKey)
	);
	}

The code above could be changed that the second snippet (DbalCollection ordering) is executed only on DballCollection instance:

if ($this->dataSource instanceof $this->dbalCollectionClass) { 
    $order = $this->dataSource->getQueryBuilder()->getClause('order');   

    if (ArraysHelper::testEmpty($order)) { 
 	$this->dataSource = $this->dataSource->orderBy( 
 		$this->prepareColumn($this->primaryKey) 
 	);
     } 
}

[radimvaculik]

@mskocik See #1062. All good?

[mskocik]

@radimvaculik Should be fine according to changes in PR. Haven't tested it yet on real code.

[radimvaculik]

@mskocik Have you tested? Should we close this issue?