Re: [QGIS-Developer] [attributetable] #15803 "Sort on Top" regression or feature ?

2017-05-10 Thread Luigi Pirelli 
1) is already in the related PR
https://github.com/boundlessgeo/QGIS/commit/cae124acea62b3d387586e3c1619144a3c12b150,
btw more controls will be added on restrict SortOrder values.
2) hmmm... constructor start with the correct AscendingMode... someone
(a plugin or the core code or a bug) do
attributeTableConfig().sortOrder(). There are some wrong
set in ComposerAttributeTable, but I don't think it's the origin of
the problem
Luigi Pirelli

**
* Boundless QGIS Support/Development: lpirelli AT boundlessgeo DOT com
* LinkedIn: https://www.linkedin.com/in/luigipirelli
* Stackexchange: http://gis.stackexchange.com/users/19667/luigi-pirelli
* GitHub: https://github.com/luipir
* Mastering QGIS 2nd Edition:
* 
https://www.packtpub.com/big-data-and-business-intelligence/mastering-qgis-second-edition
**


On 10 May 2017 at 08:53, Matthias Kuhn  wrote:
> Hi Luigi,
>
> Thanks for the detailed report. I think there are two different things to
> fix
>
> XML Loading: the static_cast should be accompanied by a validity check
> Fix the reason for 158 ever appearing (might just be a default value
> assigned to mSortOrder in qgsattributetableconfig.h)
>
> There's not much reason for flexibility here as far as I can tell, this just
> reflects the Qt::SortOrder enum. If required, we can add additional new
> flags for "SortOrderCaseSensitivity" and whatever might appear in the
> future.
>
> I hope that helps,
> Matthias
>
>
> On 5/9/17 11:24 AM, Luigi Pirelli wrote:
>
> Hi devs
>
> ***In short:
> The sortOrder value in QgsProject section:
>
> 
>
> can be any value for some reason (e.g. flexibility for the future) or
> it is a bug?
>
> I would ask to especially to Nyall that git blames is the author of
> this section:
> https://github.com/qgis/QGIS/blob/master/src/core/qgsattributetableconfig.cpp#L200
>
> ***Details:
> during hackmeeting I also worked on:
> https://issues.qgis.org/issues/15803
>
> in the team we had problem replicating the issue on QGIS 3.x so we
> discovered that the origin of the problem is in
> QgsAttributeTableConfig saved in Layer style section of the qgis
> project.
>
> here an example of a project I had with wrong values
>
>sortExpression="" sortOrder="158">
> 
>   
>   
>   
>   
> 
>   
>
> the value is casted to Qt:SortOrder enum with:
> https://github.com/qgis/QGIS/blob/master/src/core/qgsattributetableconfig.cpp#L200
>
> that is undefined in case sortOrder value does not belong to the enum
> range, but in the implementation the value is leaved to 158 (as in the
> example)
>
> This generate some side effects in the attribute table => the
> sortOnTop button does not work because of:
> https://github.com/qgis/QGIS/blob/master/src/gui/attributetable/qgsattributetablefiltermodel.cpp#L53
> and
> https://github.com/qgis/QGIS/blob/master/src/gui/attributetable/qgsattributetablefiltermodel.cpp#L57
>
> that obviously returns always false
>
> The actual patch in:
> 2.18 - https://github.com/qgis/QGIS/pull/4306
> 3.x - https://github.com/qgis/QGIS/pull/
>
> protect the attribute table from having wrong sortOrder values, but
> does not try to fix it's value in AttributeTable config that can be a
> value set for some reason by the user or other components.
>
> ***Workaroud
> a simple workaround is apply a column sort and save the project to fix
> the wrong value
>
> Luigi Pirelli
>
> **
> * Boundless QGIS Support/Development: lpirelli AT boundlessgeo DOT com
> * LinkedIn: https://www.linkedin.com/in/luigipirelli
> * Stackexchange: http://gis.stackexchange.com/users/19667/luigi-pirelli
> * GitHub: https://github.com/luipir
> * Mastering QGIS 2nd Edition:
> *
> https://www.packtpub.com/big-data-and-business-intelligence/mastering-qgis-second-edition
> **
> 
> ___
> QGIS-Developer mailing list
> QGIS-Developer@lists.osgeo.org
> List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
> Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
>
>
>
> ___
> QGIS-Developer mailing list
> QGIS-Developer@lists.osgeo.org
> List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
> Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
___
QGIS-Developer mailing list
QGIS-Developer@lists.osgeo.org
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer

Re: [QGIS-Developer] [attributetable] #15803 "Sort on Top" regression or feature ?

2017-05-10 Thread Matthias Kuhn 
Hi Luigi,

Thanks for the detailed report. I think there are two different things
to fix

 1. XML Loading: the static_cast should be accompanied by a validity check
 2. Fix the reason for 158 ever appearing (might just be a default value
assigned to mSortOrder in qgsattributetableconfig.h)

There's not much reason for flexibility here as far as I can tell, this
just reflects the Qt::SortOrder enum. If required, we can add additional
new flags for "SortOrderCaseSensitivity" and whatever might appear in
the future.

I hope that helps,
Matthias


On 5/9/17 11:24 AM, Luigi Pirelli wrote:
> Hi devs
>
> ***In short:
> The sortOrder value in QgsProject section:
>
> 
>
> can be any value for some reason (e.g. flexibility for the future) or
> it is a bug?
>
> I would ask to especially to Nyall that git blames is the author of
> this section:
> https://github.com/qgis/QGIS/blob/master/src/core/qgsattributetableconfig.cpp#L200
>
> ***Details:
> during hackmeeting I also worked on:
> https://issues.qgis.org/issues/15803
>
> in the team we had problem replicating the issue on QGIS 3.x so we
> discovered that the origin of the problem is in
> QgsAttributeTableConfig saved in Layer style section of the qgis
> project.
>
> here an example of a project I had with wrong values
>
>sortExpression="" sortOrder="158">
> 
>   
>   
>   
>   
> 
>   
>
> the value is casted to Qt:SortOrder enum with:
> https://github.com/qgis/QGIS/blob/master/src/core/qgsattributetableconfig.cpp#L200
>
> that is undefined in case sortOrder value does not belong to the enum
> range, but in the implementation the value is leaved to 158 (as in the
> example)
>
> This generate some side effects in the attribute table => the
> sortOnTop button does not work because of:
> https://github.com/qgis/QGIS/blob/master/src/gui/attributetable/qgsattributetablefiltermodel.cpp#L53
> and
> https://github.com/qgis/QGIS/blob/master/src/gui/attributetable/qgsattributetablefiltermodel.cpp#L57
>
> that obviously returns always false
>
> The actual patch in:
> 2.18 - https://github.com/qgis/QGIS/pull/4306
> 3.x - https://github.com/qgis/QGIS/pull/
>
> protect the attribute table from having wrong sortOrder values, but
> does not try to fix it's value in AttributeTable config that can be a
> value set for some reason by the user or other components.
>
> ***Workaroud
> a simple workaround is apply a column sort and save the project to fix
> the wrong value
>
> Luigi Pirelli
>
> **
> * Boundless QGIS Support/Development: lpirelli AT boundlessgeo DOT com
> * LinkedIn: https://www.linkedin.com/in/luigipirelli
> * Stackexchange: http://gis.stackexchange.com/users/19667/luigi-pirelli
> * GitHub: https://github.com/luipir
> * Mastering QGIS 2nd Edition:
> * 
> https://www.packtpub.com/big-data-and-business-intelligence/mastering-qgis-second-edition
> **
> ___
> QGIS-Developer mailing list
> QGIS-Developer@lists.osgeo.org
> List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
> Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
>

___
QGIS-Developer mailing list
QGIS-Developer@lists.osgeo.org
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer

[QGIS-Developer] [attributetable] #15803 "Sort on Top" regression or feature ?

2017-05-09 Thread Luigi Pirelli 
Hi devs

***In short:
The sortOrder value in QgsProject section:



can be any value for some reason (e.g. flexibility for the future) or
it is a bug?

I would ask to especially to Nyall that git blames is the author of
this section:
https://github.com/qgis/QGIS/blob/master/src/core/qgsattributetableconfig.cpp#L200

***Details:
during hackmeeting I also worked on:
https://issues.qgis.org/issues/15803

in the team we had problem replicating the issue on QGIS 3.x so we
discovered that the origin of the problem is in
QgsAttributeTableConfig saved in Layer style section of the qgis
project.

here an example of a project I had with wrong values

  

  
  
  
  

  

the value is casted to Qt:SortOrder enum with:
https://github.com/qgis/QGIS/blob/master/src/core/qgsattributetableconfig.cpp#L200

that is undefined in case sortOrder value does not belong to the enum
range, but in the implementation the value is leaved to 158 (as in the
example)

This generate some side effects in the attribute table => the
sortOnTop button does not work because of:
https://github.com/qgis/QGIS/blob/master/src/gui/attributetable/qgsattributetablefiltermodel.cpp#L53
and
https://github.com/qgis/QGIS/blob/master/src/gui/attributetable/qgsattributetablefiltermodel.cpp#L57

that obviously returns always false

The actual patch in:
2.18 - https://github.com/qgis/QGIS/pull/4306
3.x - https://github.com/qgis/QGIS/pull/

protect the attribute table from having wrong sortOrder values, but
does not try to fix it's value in AttributeTable config that can be a
value set for some reason by the user or other components.

***Workaroud
a simple workaround is apply a column sort and save the project to fix
the wrong value

Luigi Pirelli

**
* Boundless QGIS Support/Development: lpirelli AT boundlessgeo DOT com
* LinkedIn: https://www.linkedin.com/in/luigipirelli
* Stackexchange: http://gis.stackexchange.com/users/19667/luigi-pirelli
* GitHub: https://github.com/luipir
* Mastering QGIS 2nd Edition:
* 
https://www.packtpub.com/big-data-and-business-intelligence/mastering-qgis-second-edition
**
___
QGIS-Developer mailing list
QGIS-Developer@lists.osgeo.org
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer