Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[layout] allow sorting attribute table by field not listed in the table #36236

Merged
merged 10 commits into from
May 7, 2020
Merged

[layout] allow sorting attribute table by field not listed in the table #36236

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
fcea343
allow sorting attribute table by field not listed in the table
3nids May 6, 2020
03f4cb9
do not use a specific class for the sort columns
3nids May 6, 2020
525661c
fix backward compatibility and fix leak
3nids May 6, 2020
e7fe141
fix crash - clone the columns
3nids May 6, 2020
ee686b5
QgsLayoutTableColumn is not a QObject anymore
3nids May 6, 2020
ab4e179
regroup the 2 models within an abstract base class
3nids May 6, 2020
2b252c8
adapt test
3nids May 6, 2020
971d107
remove duplicated forward declaration*
3nids May 6, 2020
28c20cf
fix dox and layout
3nids May 6, 2020
51b0d5a
API backward compatibility fix, code layout and readability, implicit…
3nids May 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion python/core/auto_generated/layout/qgslayoutitemattributetable.sip.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@ Sets the attributes to display in the table.
to accommodate the new displayed feature attributes.
%End


void setWrapString( const QString &wrapString );
%Docstring
Sets a string to wrap the contents of the table cells by. Occurrences of this string will
Expand Down
26 changes: 25 additions & 1 deletion python/core/auto_generated/layout/qgslayouttable.sip.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ typedef QVector< QVariant > QgsLayoutTableRow;
typedef QVector< QVector< QVariant > > QgsLayoutTableContents;


typedef QVector< QgsLayoutTableColumn * > QgsLayoutTableColumns;
typedef QVector<QgsLayoutTableColumn> QgsLayoutTableColumns;

typedef QVector<QgsLayoutTableColumn> QgsLayoutTableSortColumns;




Expand Down Expand Up @@ -473,6 +476,26 @@ Replaces the columns in the table with a specified list of QgsLayoutTableColumns
:param columns: list of QgsLayoutTableColumns to show in table.

.. seealso:: :py:func:`columns`
%End

QgsLayoutTableSortColumns &sortColumns();
%Docstring
Returns a reference to the list of QgsLayoutTableSortColumns shown in the table

.. seealso:: :py:func:`setSortColumns`

.. versionadded:: 3.14
%End

void setSortColumns( const QgsLayoutTableSortColumns &sortColumns );
%Docstring
Replaces the sorting columns in the table with a specified list of QgsLayoutTableSortColumns.

:param sortColumns: list of QgsLayoutTableColumns used to sort the table.

.. seealso:: :py:func:`sortColumns`

.. versionadded:: 3.14
%End

void setCellStyle( CellStyleGroup group, const QgsLayoutTableStyle &style );
Expand Down Expand Up @@ -562,6 +585,7 @@ new data.






virtual bool calculateMaxColumnWidths();
Expand Down
27 changes: 19 additions & 8 deletions python/core/auto_generated/layout/qgslayouttablecolumn.sip.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@



class QgsLayoutTableColumn : QObject
class QgsLayoutTableColumn
{
%Docstring
Stores properties of a column for a QgsLayoutTable. Some properties of aQgsLayoutTableColumn
are applicable only in certain contexts. For instance, the attribute and setAttribute methods only
have an effect for QgsLayoutItemAttributeTables, and have no effect for QgsLayoutItemTextTables.
Stores properties of a column for a QgsLayoutTable.
Some properties of a QgsLayoutTableColumn are applicable only in certain contexts.
For instance, the attribute and setAttribute methods only have an effect
for QgsLayoutItemAttributeTables, and have no effect for QgsLayoutItemTextTables.

.. versionadded:: 3.0
%End
Expand Down Expand Up @@ -174,7 +175,7 @@ is part of a QgsLayoutItemAttributeTable and when sortByRank() is > 0.
.. seealso:: :py:func:`setSortByRank`
%End

int sortByRank() const;
int sortByRank() const /Deprecated/;
%Docstring
Returns the sort rank for the column. If the sort rank is > 0 then the column
will be sorted in the table. The sort rank specifies the priority given to the
Expand All @@ -191,9 +192,12 @@ If sort rank is <= 0 then the column is not being sorted.
.. seealso:: :py:func:`setSortByRank`

.. seealso:: :py:func:`sortOrder`

.. deprecated:: QGIS 3.14
the order is now hold in a dedicated model
%End

void setSortByRank( int rank );
void setSortByRank( int rank ) /Deprecated/;
%Docstring
Sets the sort ``rank`` for the column. If the sort rank is > 0 then the column
will be sorted in the table. The sort rank specifies the priority given to the
Expand All @@ -209,17 +213,24 @@ If the sort ``rank`` is <= 0 then the column is not being sorted.
.. seealso:: :py:func:`sortByRank`

.. seealso:: :py:func:`setSortOrder`

.. deprecated:: QGIS 3.14
the order is now hold in a dedicated model
%End

QgsLayoutTableColumn *clone() /Factory/;
QgsLayoutTableColumn *clone() /Deprecated,Factory/;
%Docstring
Creates a duplicate column which is a deep copy of this column.

:return: a new QgsLayoutTableColumn with same properties as this column.

.. deprecated:: QGIS 3.14
use a copy instead
%End

};
bool operator==( const QgsLayoutTableColumn &other );

};
/************************************************************************
* This file has been generated automatically from *
* *
Expand Down
28 changes: 19 additions & 9 deletions src/core/layout/qgscompositionconverter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1429,6 +1429,8 @@ bool QgsCompositionConverter::readTableXml( QgsLayoutItemAttributeTable *layoutI

//restore column specifications
layoutItem->mColumns.clear();
layoutItem->mSortColumns.clear();

QDomNodeList columnsList = itemElem.elementsByTagName( QStringLiteral( "displayColumns" ) );
if ( !columnsList.isEmpty() )
{
Expand All @@ -1437,14 +1439,14 @@ bool QgsCompositionConverter::readTableXml( QgsLayoutItemAttributeTable *layoutI
for ( int i = 0; i < columnEntryList.size(); ++i )
{
QDomElement columnElem = columnEntryList.at( i ).toElement();
QgsLayoutTableColumn *column = new QgsLayoutTableColumn;
column->mHAlignment = static_cast< Qt::AlignmentFlag >( columnElem.attribute( QStringLiteral( "hAlignment" ), QString::number( Qt::AlignLeft ) ).toInt() );
column->mVAlignment = static_cast< Qt::AlignmentFlag >( columnElem.attribute( QStringLiteral( "vAlignment" ), QString::number( Qt::AlignVCenter ) ).toInt() );
column->mHeading = columnElem.attribute( QStringLiteral( "heading" ), QString() );
column->mAttribute = columnElem.attribute( QStringLiteral( "attribute" ), QString() );
column->mSortByRank = columnElem.attribute( QStringLiteral( "sortByRank" ), QStringLiteral( "0" ) ).toInt();
column->mSortOrder = static_cast< Qt::SortOrder >( columnElem.attribute( QStringLiteral( "sortOrder" ), QString::number( Qt::AscendingOrder ) ).toInt() );
column->mWidth = columnElem.attribute( QStringLiteral( "width" ), QStringLiteral( "0.0" ) ).toDouble();
QgsLayoutTableColumn column;
column.mHAlignment = static_cast< Qt::AlignmentFlag >( columnElem.attribute( QStringLiteral( "hAlignment" ), QString::number( Qt::AlignLeft ) ).toInt() );
column.mVAlignment = static_cast< Qt::AlignmentFlag >( columnElem.attribute( QStringLiteral( "vAlignment" ), QString::number( Qt::AlignVCenter ) ).toInt() );
column.mHeading = columnElem.attribute( QStringLiteral( "heading" ), QString() );
column.mAttribute = columnElem.attribute( QStringLiteral( "attribute" ), QString() );
column.mSortByRank = columnElem.attribute( QStringLiteral( "sortByRank" ), QStringLiteral( "0" ) ).toInt();
column.mSortOrder = static_cast< Qt::SortOrder >( columnElem.attribute( QStringLiteral( "sortOrder" ), QString::number( Qt::AscendingOrder ) ).toInt() );
column.mWidth = columnElem.attribute( QStringLiteral( "width" ), QStringLiteral( "0.0" ) ).toDouble();

QDomNodeList bgColorList = columnElem.elementsByTagName( QStringLiteral( "backgroundColor" ) );
if ( !bgColorList.isEmpty() )
Expand All @@ -1458,10 +1460,18 @@ bool QgsCompositionConverter::readTableXml( QgsLayoutItemAttributeTable *layoutI
bgAlpha = bgColorElem.attribute( QStringLiteral( "alpha" ) ).toDouble( &alphaOk );
if ( redOk && greenOk && blueOk && alphaOk )
{
column->mBackgroundColor = QColor( bgRed, bgGreen, bgBlue, bgAlpha );
column.mBackgroundColor = QColor( bgRed, bgGreen, bgBlue, bgAlpha );
}
}
layoutItem->mColumns.append( column );

// sorting columns are now (QGIS 3.14+) handled in a dedicated list
Q_NOWARN_DEPRECATED_PUSH
for ( const QgsLayoutTableColumn &col : qgis::as_const( layoutItem->mColumns ) )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would be easier to read/safer if you enclosed the loop contents in { / }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will switch back to std::copy_if then

if ( col.sortByRank() > 0 )
layoutItem->mSortColumns.append( col );
std::sort( layoutItem->mSortColumns.begin(), layoutItem->mSortColumns.end(), []( const QgsLayoutTableColumn & a, const QgsLayoutTableColumn & b ) {return a.sortByRank() < b.sortByRank();} );
Q_NOWARN_DEPRECATED_POP
}
}

Expand Down
80 changes: 22 additions & 58 deletions src/core/layout/qgslayoutitemattributetable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,19 +170,19 @@ void QgsLayoutItemAttributeTable::resetColumns()
}

//remove existing columns
qDeleteAll( mColumns );
mColumns.clear();
mSortColumns.clear();

//rebuild columns list from vector layer fields
int idx = 0;
const QgsFields sourceFields = source->fields();
for ( const auto &field : sourceFields )
{
QString currentAlias = source->attributeDisplayName( idx );
std::unique_ptr< QgsLayoutTableColumn > col = qgis::make_unique< QgsLayoutTableColumn >();
col->setAttribute( field.name() );
col->setHeading( currentAlias );
mColumns.append( col.release() );
QgsLayoutTableColumn col;
col.setAttribute( field.name() );
col.setHeading( currentAlias );
mColumns.append( col );
idx++;
}
}
Expand Down Expand Up @@ -319,7 +319,6 @@ void QgsLayoutItemAttributeTable::setDisplayedFields( const QStringList &fields,
}

//rebuild columns list, taking only fields contained in supplied list
qDeleteAll( mColumns );
mColumns.clear();

const QgsFields layerFields = source->fields();
Expand All @@ -333,10 +332,10 @@ void QgsLayoutItemAttributeTable::setDisplayedFields( const QStringList &fields,
continue;

QString currentAlias = source->attributeDisplayName( attrIdx );
std::unique_ptr< QgsLayoutTableColumn > col = qgis::make_unique< QgsLayoutTableColumn >();
col->setAttribute( layerFields.at( attrIdx ).name() );
col->setHeading( currentAlias );
mColumns.append( col.release() );
QgsLayoutTableColumn col;
col.setAttribute( layerFields.at( attrIdx ).name() );
col.setHeading( currentAlias );
mColumns.append( col );
}
}
else
Expand All @@ -346,10 +345,10 @@ void QgsLayoutItemAttributeTable::setDisplayedFields( const QStringList &fields,
for ( const QgsField &field : layerFields )
{
QString currentAlias = source->attributeDisplayName( idx );
std::unique_ptr< QgsLayoutTableColumn > col = qgis::make_unique< QgsLayoutTableColumn >();
col->setAttribute( field.name() );
col->setHeading( currentAlias );
mColumns.append( col.release() );
QgsLayoutTableColumn col;
col.setAttribute( field.name() );
col.setHeading( currentAlias );
mColumns.append( col );
idx++;
}
}
Expand All @@ -368,16 +367,16 @@ void QgsLayoutItemAttributeTable::restoreFieldAliasMap( const QMap<int, QString>
return;
}

for ( QgsLayoutTableColumn *column : qgis::as_const( mColumns ) )
for ( QgsLayoutTableColumn &column : mColumns )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that you're NOT allowed to modify a qt container inside a range based loop. Can you confirm that this is safe?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct, I will fix this

{
int attrIdx = source->fields().lookupField( column->attribute() );
int attrIdx = source->fields().lookupField( column.attribute() );
if ( map.contains( attrIdx ) )
{
column->setHeading( map.value( attrIdx ) );
column.setHeading( map.value( attrIdx ) );
}
else
{
column->setHeading( source->attributeDisplayName( attrIdx ) );
column.setHeading( source->attributeDisplayName( attrIdx ) );
}
}
}
Expand Down Expand Up @@ -480,10 +479,9 @@ bool QgsLayoutItemAttributeTable::getTableContents( QgsLayoutTableContents &cont
req.setFilterFid( atlasFeature.id() );
}

QVector< QPair<int, bool> > sortColumns = sortAttributes();
for ( int i = sortColumns.size() - 1; i >= 0; --i )
for ( const QgsLayoutTableColumn &column : qgis::as_const( mSortColumns ) )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's an api break here -- if someone is using the old api to set a list of columns with their sort order contained, then that won't be respected here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then setColumns would be used, right?
When using setColumns, I can check if a sorting order is provided and call setSortColumns at the same time.
Works for you?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about that too- but wouldn't it be hard to differentiate between a call to setColumns using the old API, vs correct use of the new api and calling setSortColumns and then later setColumns?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, if you correctly use the API there is no sorting info (rank) in the list when calling setColumns. If there is, then you're using the old API and we call setSortColumns.

{
req = req.addOrderBy( mColumns.at( sortColumns.at( i ).first )->attribute(), sortColumns.at( i ).second );
req = req.addOrderBy( column.attribute(), column.sortOrder() == Qt::AscendingOrder );
}

QgsFeature f;
Expand Down Expand Up @@ -549,9 +547,9 @@ bool QgsLayoutItemAttributeTable::getTableContents( QgsLayoutTableContents &cont
QgsLayoutTableRow rowContents;
rowContents.reserve( mColumns.count() );

for ( QgsLayoutTableColumn *column : qgis::as_const( mColumns ) )
for ( const QgsLayoutTableColumn &column : qgis::as_const( mColumns ) )
{
int idx = layer->fields().lookupField( column->attribute() );
int idx = layer->fields().lookupField( column.attribute() );

QgsConditionalStyle style;

Expand All @@ -574,7 +572,7 @@ bool QgsLayoutItemAttributeTable::getTableContents( QgsLayoutTableContents &cont
else
{
// Lets assume it's an expression
std::unique_ptr< QgsExpression > expression = qgis::make_unique< QgsExpression >( column->attribute() );
std::unique_ptr< QgsExpression > expression = qgis::make_unique< QgsExpression >( column.attribute() );
context.lastScope()->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "row_number" ), counter + 1, true ) );
expression->prepare( &context );
QVariant value = expression->evaluate( &context );
Expand Down Expand Up @@ -716,45 +714,11 @@ void QgsLayoutItemAttributeTable::removeLayer( const QString &layerId )
{
mVectorLayer.setLayer( nullptr );
//remove existing columns
qDeleteAll( mColumns );
mColumns.clear();
}
}
}

static bool columnsBySortRank( QPair<int, QgsLayoutTableColumn * > a, QPair<int, QgsLayoutTableColumn * > b )
{
return a.second->sortByRank() < b.second->sortByRank();
}

QVector<QPair<int, bool> > QgsLayoutItemAttributeTable::sortAttributes() const
{
//generate list of all sorted columns
QVector< QPair<int, QgsLayoutTableColumn * > > sortedColumns;
int idx = 0;
for ( QgsLayoutTableColumn *column : mColumns )
{
if ( column->sortByRank() > 0 )
{
sortedColumns.append( qMakePair( idx, column ) );
}
idx++;
}

//sort columns by rank
std::sort( sortedColumns.begin(), sortedColumns.end(), columnsBySortRank );

//generate list of column index, bool for sort direction (to match 2.0 api)
QVector<QPair<int, bool> > attributesBySortRank;
attributesBySortRank.reserve( sortedColumns.size() );
for ( auto &column : qgis::as_const( sortedColumns ) )
{
attributesBySortRank.append( qMakePair( column.first,
column.second->sortOrder() == Qt::AscendingOrder ) );
}
return attributesBySortRank;
}

void QgsLayoutItemAttributeTable::setWrapString( const QString &wrapString )
{
if ( wrapString == mWrapString )
Expand Down
9 changes: 0 additions & 9 deletions src/core/layout/qgslayoutitemattributetable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,15 +256,6 @@ class CORE_EXPORT QgsLayoutItemAttributeTable: public QgsLayoutTable
*/
void setDisplayedFields( const QStringList &fields, bool refresh = true );

/**
* Returns the attributes used to sort the table's features.
* \returns a QList of integer/bool pairs, where the integer refers to the attribute index and
* the bool to the sort order for the attribute. If TRUE the attribute is sorted ascending,
* if FALSE, the attribute is sorted in descending order.
* \note not available in Python bindings
*/
QVector< QPair<int, bool> > sortAttributes() const SIP_SKIP;

/**
* Sets a string to wrap the contents of the table cells by. Occurrences of this string will
* be replaced by a line break.
Expand Down
Loading