Skip to content

Commit 7e711f1

Browse files
elpasoelpaso
committedNov 29, 2017
[bugfix] Fix broken bookmarks
Fixes #17539 I can only create one spatial Bookmark in current master In fact there were other bugs (like loosing SRID when converting to "in project").
1 parent a316530 commit 7e711f1

File tree

2 files changed

+86
-71
lines changed

2 files changed

+86
-71
lines changed
 

‎src/app/qgsbookmarks.cpp

Lines changed: 83 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include <QMessageBox>
2929
#include <QSqlError>
3030
#include <QSqlQuery>
31+
#include <QSqlRecord>
3132
#include <QModelIndex>
3233
#include <QAbstractTableModel>
3334
#include <QToolButton>
@@ -138,10 +139,6 @@ void QgsBookmarks::addClicked()
138139
QgsMapCanvas *canvas = QgisApp::instance()->mapCanvas();
139140
Q_ASSERT( canvas );
140141

141-
QSqlQuery query( mQgisModel->database() );
142-
query.prepare( "INSERT INTO tbl_bookmarks(bookmark_id,name,project_name,xmin,ymin,xmax,ymax,projection_srid)"
143-
" VALUES (NULL,:name,:project_name,:xmin,:ymin,:xmax,:ymax,:projection_srid)" );
144-
145142
QString projStr( QLatin1String( "" ) );
146143
if ( QgsProject::instance() )
147144
{
@@ -156,14 +153,16 @@ void QgsBookmarks::addClicked()
156153
}
157154
}
158155

159-
query.bindValue( QStringLiteral( ":name" ), tr( "New bookmark" ) );
160-
query.bindValue( QStringLiteral( ":project_name" ), projStr );
161-
query.bindValue( QStringLiteral( ":xmin" ), canvas->extent().xMinimum() );
162-
query.bindValue( QStringLiteral( ":ymin" ), canvas->extent().yMinimum() );
163-
query.bindValue( QStringLiteral( ":xmax" ), canvas->extent().xMaximum() );
164-
query.bindValue( QStringLiteral( ":ymax" ), canvas->extent().yMaximum() );
165-
query.bindValue( QStringLiteral( ":projection_srid" ), QVariant::fromValue( canvas->mapSettings().destinationCrs().srsid() ) );
166-
if ( query.exec() )
156+
QSqlRecord record = mQgisModel->record();
157+
record.setValue( 1, QVariant( tr( "New bookmark" ) ) );
158+
record.setValue( 2, QVariant( projStr ) );
159+
record.setValue( 3, QVariant( canvas->extent().xMinimum() ) );
160+
record.setValue( 4, QVariant( canvas->extent().yMinimum() ) );
161+
record.setValue( 5, QVariant( canvas->extent().xMaximum() ) );
162+
record.setValue( 6, QVariant( canvas->extent().yMaximum() ) );
163+
record.setValue( 7, QVariant::fromValue( canvas->mapSettings().destinationCrs().srsid() ) );
164+
165+
if ( mQgisModel->insertRecord( -1, record ) )
167166
{
168167
mQgisModel->setSort( 0, Qt::AscendingOrder );
169168
mQgisModel->select();
@@ -174,8 +173,8 @@ void QgsBookmarks::addClicked()
174173
else
175174
{
176175
QMessageBox::warning( this, tr( "Error" ), tr( "Unable to create the bookmark.\nDriver: %1\nDatabase: %2" )
177-
.arg( query.lastError().driverText(),
178-
query.lastError().databaseText() ) );
176+
.arg( mQgisModel->database().lastError().driverText(),
177+
mQgisModel->database().lastError().databaseText() ) );
179178
}
180179
}
181180

@@ -408,13 +407,16 @@ int QgsProjectBookmarksTableModel::rowCount( const QModelIndex &parent ) const
408407
int QgsProjectBookmarksTableModel::columnCount( const QModelIndex &parent ) const
409408
{
410409
Q_UNUSED( parent );
411-
return 7;
410+
return 8;
412411
}
413412

414413
QVariant QgsProjectBookmarksTableModel::data( const QModelIndex &index, int role ) const
415414
{
416-
Q_UNUSED( role );
417-
Q_ASSERT( role == Qt::DisplayRole );
415+
416+
if ( role != Qt::DisplayRole && role != Qt::EditRole )
417+
{
418+
return QVariant();
419+
}
418420

419421
switch ( index.column() )
420422
{
@@ -431,7 +433,7 @@ QVariant QgsProjectBookmarksTableModel::data( const QModelIndex &index, int role
431433
case 6:
432434
return QgsProject::instance()->readDoubleEntry( QStringLiteral( "Bookmarks" ), QStringLiteral( "/Row-%1/MaxY" ).arg( index.row() ) );
433435
case 7:
434-
return QgsProject::instance()->readNumEntry( QStringLiteral( "Bookmarks" ), QStringLiteral( "/Row-%1/SRID" ).arg( index.row() ) );
436+
return QgsProject::instance()->readNumEntry( QStringLiteral( "Bookmarks" ), QStringLiteral( "/Row-%1/SRID" ).arg( index.row() ) );
435437
default:
436438
return QVariant();
437439
}
@@ -474,14 +476,17 @@ bool QgsProjectBookmarksTableModel::insertRows( int row, int count, const QModel
474476
{
475477
Q_UNUSED( row );
476478
Q_UNUSED( parent );
479+
beginInsertRows( parent, row, row + count );
477480

478-
return QgsProject::instance()->writeEntry( QStringLiteral( "Bookmarks" ), QStringLiteral( "/count" ), QgsProject::instance()->readNumEntry( QStringLiteral( "Bookmarks" ), QStringLiteral( "/count" ) ) + count );
481+
bool result = QgsProject::instance()->writeEntry( QStringLiteral( "Bookmarks" ), QStringLiteral( "/count" ), QgsProject::instance()->readNumEntry( QStringLiteral( "Bookmarks" ), QStringLiteral( "/count" ) ) + count );
482+
endInsertRows();
483+
return result;
479484
}
480485

481486
bool QgsProjectBookmarksTableModel::removeRows( int row, int count, const QModelIndex &parent )
482487
{
483488
Q_UNUSED( parent );
484-
489+
beginRemoveRows( parent, row, row + count );
485490
for ( int newRow = row ; newRow < rowCount() - count ; newRow++ )
486491
{
487492
for ( int column = 0; column < columnCount() ; column++ )
@@ -495,10 +500,11 @@ bool QgsProjectBookmarksTableModel::removeRows( int row, int count, const QModel
495500
}
496501

497502
QgsProject::instance()->writeEntry( QStringLiteral( "Bookmarks" ), QStringLiteral( "/count" ), QgsProject::instance()->readNumEntry( QStringLiteral( "Bookmarks" ), QStringLiteral( "/count" ) ) - count );
498-
503+
endRemoveRows();
499504
return true;
500505
}
501506

507+
502508
QgsMergedBookmarksTableModel::QgsMergedBookmarksTableModel( QAbstractTableModel &qgisTableModel, QAbstractTableModel &projectTableModel, QTreeView *treeView )
503509
: mQgisTableModel( qgisTableModel )
504510
, mProjectTableModel( projectTableModel )
@@ -512,19 +518,20 @@ QgsMergedBookmarksTableModel::QgsMergedBookmarksTableModel( QAbstractTableModel
512518
connect(
513519
&mQgisTableModel, &QAbstractTableModel::layoutChanged,
514520
this, &QgsMergedBookmarksTableModel::allLayoutChanged );
515-
connect(
516-
&mQgisTableModel, &QAbstractTableModel::dataChanged,
517-
this, &QgsMergedBookmarksTableModel::qgisDataChanged );
521+
518522
connect(
519523
&mQgisTableModel, &QAbstractTableModel::rowsInserted,
520524
this, &QgsMergedBookmarksTableModel::allLayoutChanged );
525+
521526
connect(
522527
&mQgisTableModel, &QAbstractTableModel::rowsRemoved,
523528
this, &QgsMergedBookmarksTableModel::allLayoutChanged );
524529

525530
connect(
526531
&projectTableModel, &QAbstractTableModel::layoutChanged,
527532
this, &QgsMergedBookmarksTableModel::allLayoutChanged );
533+
534+
528535
}
529536

530537
int QgsMergedBookmarksTableModel::rowCount( const QModelIndex &parent ) const
@@ -539,33 +546,37 @@ int QgsMergedBookmarksTableModel::columnCount( const QModelIndex &parent ) const
539546

540547
QVariant QgsMergedBookmarksTableModel::data( const QModelIndex &index, int role ) const
541548
{
542-
// is project or application
543-
if ( index.column() == mQgisTableModel.columnCount() )
544-
{
545-
if ( role == Qt::CheckStateRole )
546-
{
547-
return index.row() < mQgisTableModel.rowCount() ? Qt::Unchecked : Qt::Checked;
548-
}
549-
else
550-
{
551-
return QVariant();
552-
}
553-
}
554-
if ( index.row() < mQgisTableModel.rowCount() )
549+
QVariant value;
550+
// Is it checkbox column?
551+
if ( index.column() == mQgisTableModel.columnCount() && role == Qt::CheckStateRole )
555552
{
556-
return mQgisTableModel.data( index, role );
553+
value = index.row() < mQgisTableModel.rowCount() ? Qt::Unchecked : Qt::Checked;
557554
}
558555
else
559556
{
560-
if ( role == Qt::DisplayRole || role == Qt::EditRole )
557+
// Is it a SQLite stored entry ?
558+
if ( index.row() < mQgisTableModel.rowCount() )
561559
{
562-
return mProjectTableModel.data( this->index( index.row() - mQgisTableModel.rowCount(), index.column() ), role );
560+
value = mQgisTableModel.data( index, role );
563561
}
564-
else
562+
else // ... it is a project stored bookmark
565563
{
566-
return mQgisTableModel.data( this->index( 0, index.column() ), role );
564+
if ( role == Qt::DisplayRole || role == Qt::EditRole )
565+
{
566+
value = mProjectTableModel.data( this->index( index.row() - mQgisTableModel.rowCount(), index.column() ), role );
567+
}
568+
else // Default roles from base model
569+
{
570+
value = mQgisTableModel.data( this->index( 0, index.column() ), role );
571+
}
572+
}
573+
// Is it the projection column ?
574+
if ( ( role == Qt::DisplayRole || role == Qt::EditRole ) && index.column( ) == mQgisTableModel.columnCount() - 1 )
575+
{
576+
value = QgsCoordinateReferenceSystem::fromSrsId( value.toInt( ) ).authid( );
567577
}
568578
}
579+
return value;
569580
}
570581

571582
bool QgsMergedBookmarksTableModel::setData( const QModelIndex &index, const QVariant &value, int role )
@@ -575,13 +586,15 @@ bool QgsMergedBookmarksTableModel::setData( const QModelIndex &index, const QVar
575586
{
576587
if ( index.row() < mQgisTableModel.rowCount() )
577588
{
589+
// Move from SQLite storage to project
578590
moveBookmark( mQgisTableModel, mProjectTableModel, index.row() );
579591
mTreeView->scrollTo( this->index( rowCount() - 1, 1 ) );
580592
mTreeView->setCurrentIndex( this->index( rowCount() - 1, 1 ) );
581593
mTreeView->selectionModel()->select( this->index( rowCount() - 1, 1 ), QItemSelectionModel::Rows );
582594
}
583595
else
584596
{
597+
//Move from project to SQLite storage
585598
moveBookmark( mProjectTableModel, mQgisTableModel, index.row() - mQgisTableModel.rowCount() );
586599
mTreeView->scrollTo( this->index( mQgisTableModel.rowCount() - 1, 1 ) );
587600
mTreeView->setCurrentIndex( this->index( mQgisTableModel.rowCount() - 1, 1 ) );
@@ -615,23 +628,31 @@ Qt::ItemFlags QgsMergedBookmarksTableModel::flags( const QModelIndex &index ) co
615628
}
616629
else
617630
{
618-
flags |= Qt::ItemIsEditable;
631+
// Skip projection: not editable!
632+
if ( index.column() != mQgisTableModel.columnCount() - 1 )
633+
flags |= Qt::ItemIsEditable;
619634
}
620635
return flags;
621636
}
622637

623638
bool QgsMergedBookmarksTableModel::removeRows( int row, int count, const QModelIndex &parent )
624639
{
625640
Q_ASSERT( count == 1 );
626-
641+
bool result;
642+
beginRemoveRows( parent, row, row + count );
627643
if ( row < mQgisTableModel.rowCount() )
628644
{
629-
return mQgisTableModel.removeRows( row, count, parent );
645+
QSqlTableModel *qgisModel = static_cast<QSqlTableModel *>( &mQgisTableModel );
646+
Q_ASSERT( qgisModel );
647+
result = qgisModel->removeRows( row, count, parent );
648+
qgisModel->select();
630649
}
631650
else
632651
{
633-
return mProjectTableModel.removeRows( row - mQgisTableModel.rowCount(), count, parent );
652+
result = mProjectTableModel.removeRows( row - mQgisTableModel.rowCount(), count, parent );
634653
}
654+
endRemoveRows();
655+
return result;
635656
}
636657

637658
QVariant QgsMergedBookmarksTableModel::headerData( int section, Qt::Orientation orientation, int role ) const
@@ -666,34 +687,35 @@ void QgsMergedBookmarksTableModel::moveBookmark( QAbstractTableModel &modelFrom,
666687
modelTo.insertRow( -1 );
667688
for ( int column = 1 ; column < modelFrom.columnCount() ; column++ )
668689
{
690+
Q_ASSERT( modelTo.index( modelTo.rowCount() - 1, column ).isValid( ) );
669691
modelTo.setData(
670692
modelTo.index( modelTo.rowCount() - 1, column ),
671693
modelFrom.data( modelFrom.index( row, column ) ) );
672694
}
695+
qgisModel = dynamic_cast<QSqlTableModel *>( &modelFrom );
696+
Q_ASSERT( qgisModel );
697+
qgisModel->removeRows( row, 1 );
698+
qgisModel->select();
673699
}
674700
else
675701
{
676-
QSqlQuery query( "INSERT INTO tbl_bookmarks(bookmark_id,name,project_name,xmin,ymin,xmax,ymax,projection_srid)"
677-
" VALUES (NULL,:name,:project_name,:xmin,:ymin,:xmax,:ymax,:projection_srid)",
678-
qgisModel->database() );
702+
QSqlRecord record = qgisModel->record();
703+
record.setValue( 1, modelFrom.data( modelFrom.index( row, 1 ) ).toString() );
704+
record.setValue( 2, modelFrom.data( modelFrom.index( row, 2 ) ).toString() );
705+
record.setValue( 3, modelFrom.data( modelFrom.index( row, 3 ) ).toDouble() );
706+
record.setValue( 4, modelFrom.data( modelFrom.index( row, 4 ) ).toDouble() );
707+
record.setValue( 5, modelFrom.data( modelFrom.index( row, 5 ) ).toDouble() );
708+
record.setValue( 6, modelFrom.data( modelFrom.index( row, 6 ) ).toDouble() );
709+
record.setValue( 7, modelFrom.data( modelFrom.index( row, 7 ) ).toInt() );
679710

680-
query.bindValue( QStringLiteral( ":name" ), modelFrom.data( modelFrom.index( row, 1 ) ).toString() );
681-
query.bindValue( QStringLiteral( ":project_name" ), modelFrom.data( modelFrom.index( row, 2 ) ).toString() );
682-
query.bindValue( QStringLiteral( ":xmin" ), modelFrom.data( modelFrom.index( row, 3 ) ).toDouble() );
683-
query.bindValue( QStringLiteral( ":ymin" ), modelFrom.data( modelFrom.index( row, 4 ) ).toDouble() );
684-
query.bindValue( QStringLiteral( ":xmax" ), modelFrom.data( modelFrom.index( row, 5 ) ).toDouble() );
685-
query.bindValue( QStringLiteral( ":ymax" ), modelFrom.data( modelFrom.index( row, 6 ) ).toDouble() );
686-
query.bindValue( QStringLiteral( ":projection_srid" ), modelFrom.data( modelFrom.index( row, 7 ) ).toInt() );
687-
688-
if ( !query.exec() )
711+
if ( ! qgisModel->insertRecord( -1, record ) )
689712
{
690713
QgsDebugMsg( QString( "Could not move bookmark: %1" )
691-
.arg( query.lastError().text() ) );
714+
.arg( qgisModel->database().lastError().text() ) );
692715
return;
693716
}
694717
qgisModel->setSort( 0, Qt::AscendingOrder );
695718
qgisModel->select();
719+
modelFrom.removeRows( row, 1 );
696720
}
697-
698-
modelFrom.removeRow( row );
699721
}

‎src/app/qgsbookmarks.h

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,17 +85,10 @@ class QgsMergedBookmarksTableModel: public QAbstractTableModel
8585

8686
private slots:
8787
void projectRead() { mProjectOpen = true; }
88-
void allLayoutChanged() { emit layoutChanged(); }
89-
void qgisDataChanged( const QModelIndex &topLeft, const QModelIndex &bottomRight )
88+
void allLayoutChanged()
9089
{
91-
emit dataChanged( topLeft, bottomRight );
92-
};
93-
void projectDataChanged( const QModelIndex &topLeft, const QModelIndex &bottomRight )
94-
{
95-
emit dataChanged(
96-
index( topLeft.row() + mQgisTableModel.rowCount(), topLeft.column() ),
97-
index( bottomRight.row() + mQgisTableModel.rowCount(), bottomRight.column() ) );
98-
};
90+
emit layoutChanged();
91+
}
9992
};
10093

10194

0 commit comments

Comments
 (0)
Please sign in to comment.