diff --git a/src/presentation/artifacteditormodel.h b/src/presentation/artifacteditormodel.h --- a/src/presentation/artifacteditormodel.h +++ b/src/presentation/artifacteditormodel.h @@ -49,6 +49,8 @@ Q_PROPERTY(QDateTime dueDate READ dueDate WRITE setDueDate NOTIFY dueDateChanged) Q_PROPERTY(QString delegateText READ delegateText NOTIFY delegateTextChanged) Q_PROPERTY(bool hasTaskProperties READ hasTaskProperties NOTIFY hasTaskPropertiesChanged) + Q_PROPERTY(bool editingInProgress READ editingInProgress WRITE setEditingInProgress) + public: typedef std::function SaveFunction; typedef std::function DelegateFunction; @@ -76,14 +78,18 @@ static int autoSaveDelay(); + bool editingInProgress() const; + public slots: void setText(const QString &text); void setTitle(const QString &title); void setDone(bool done); void setStartDate(const QDateTime &start); void setDueDate(const QDateTime &due); void delegate(const QString &name, const QString &email); + void setEditingInProgress(bool editingInProgress); + signals: void artifactChanged(const Domain::Artifact::Ptr &artifact); void hasTaskPropertiesChanged(bool hasTaskProperties); @@ -107,6 +113,11 @@ private: void setSaveNeeded(bool needed); bool isSaveNeeded() const; + void applyNewText(const QString &text); + void applyNewTitle(const QString &title); + void applyNewDone(bool done); + void applyNewStartDate(const QDateTime &start); + void applyNewDueDate(const QDateTime &due); Domain::Artifact::Ptr m_artifact; SaveFunction m_saveFunction; @@ -121,6 +132,7 @@ QTimer *m_saveTimer; bool m_saveNeeded; + bool m_editingInProgress; }; } diff --git a/src/presentation/artifacteditormodel.cpp b/src/presentation/artifacteditormodel.cpp --- a/src/presentation/artifacteditormodel.cpp +++ b/src/presentation/artifacteditormodel.cpp @@ -35,7 +35,8 @@ : QObject(parent), m_done(false), m_saveTimer(new QTimer(this)), - m_saveNeeded(false) + m_saveNeeded(false), + m_editingInProgress(false) { m_saveTimer->setSingleShot(true); m_saveTimer->setInterval(autoSaveDelay()); @@ -160,43 +161,48 @@ return 500; } +bool ArtifactEditorModel::editingInProgress() const +{ + return m_editingInProgress; +} + void ArtifactEditorModel::setText(const QString &text) { if (m_text == text) return; - onTextChanged(text); + applyNewText(text); setSaveNeeded(true); } void ArtifactEditorModel::setTitle(const QString &title) { if (m_title == title) return; - onTitleChanged(title); + applyNewTitle(title); setSaveNeeded(true); } void ArtifactEditorModel::setDone(bool done) { if (m_done == done) return; - onDoneChanged(done); + applyNewDone(done); setSaveNeeded(true); } void ArtifactEditorModel::setStartDate(const QDateTime &start) { if (m_start == start) return; - onStartDateChanged(start); + applyNewStartDate(start); setSaveNeeded(true); } void ArtifactEditorModel::setDueDate(const QDateTime &due) { if (m_due == due) return; - onDueDateChanged(due); + applyNewDueDate(due); setSaveNeeded(true); } @@ -208,34 +214,39 @@ m_delegateFunction(task, delegate); } +void ArtifactEditorModel::setEditingInProgress(bool editing) +{ + m_editingInProgress = editing; +} + void ArtifactEditorModel::onTextChanged(const QString &text) { - m_text = text; - emit textChanged(m_text); + if (!m_editingInProgress) + applyNewText(text); } void ArtifactEditorModel::onTitleChanged(const QString &title) { - m_title = title; - emit titleChanged(m_title); + if (!m_editingInProgress) + applyNewTitle(title); } void ArtifactEditorModel::onDoneChanged(bool done) { - m_done = done; - emit doneChanged(m_done); + if (!m_editingInProgress) + applyNewDone(done); } void ArtifactEditorModel::onStartDateChanged(const QDateTime &start) { - m_start = start; - emit startDateChanged(m_start); + if (!m_editingInProgress) + applyNewStartDate(start); } void ArtifactEditorModel::onDueDateChanged(const QDateTime &due) { - m_due = due; - emit dueDateChanged(m_due); + if (!m_editingInProgress) + applyNewDueDate(due); } void ArtifactEditorModel::onDelegateChanged(const Domain::Task::Delegate &delegate) @@ -280,3 +291,33 @@ { return m_saveNeeded; } + +void ArtifactEditorModel::applyNewText(const QString &text) +{ + m_text = text; + emit textChanged(m_text); +} + +void ArtifactEditorModel::applyNewTitle(const QString &title) +{ + m_title = title; + emit titleChanged(m_title); +} + +void ArtifactEditorModel::applyNewDone(bool done) +{ + m_done = done; + emit doneChanged(m_done); +} + +void ArtifactEditorModel::applyNewStartDate(const QDateTime &start) +{ + m_start = start; + emit startDateChanged(m_start); +} + +void ArtifactEditorModel::applyNewDueDate(const QDateTime &due) +{ + m_due = due; + emit dueDateChanged(m_due); +} diff --git a/src/widgets/editorview.h b/src/widgets/editorview.h --- a/src/widgets/editorview.h +++ b/src/widgets/editorview.h @@ -64,6 +64,9 @@ void dueDateChanged(const QDateTime &due); void doneChanged(bool done); +protected: + bool eventFilter(QObject *watched, QEvent *event) Q_DECL_OVERRIDE; + private slots: void onArtifactChanged(); void onHasTaskPropertiesChanged(); diff --git a/src/widgets/editorview.cpp b/src/widgets/editorview.cpp --- a/src/widgets/editorview.cpp +++ b/src/widgets/editorview.cpp @@ -69,6 +69,12 @@ ui->delegateLabel->setVisible(false); ui->taskGroup->setVisible(false); + ui->textEdit->installEventFilter(this); + ui->startDateEdit->installEventFilter(this); + ui->dueDateEdit->installEventFilter(this); + ui->doneButton->installEventFilter(this); + m_delegateEdit->installEventFilter(this); + connect(ui->textEdit, &QPlainTextEdit::textChanged, this, &EditorView::onTextEditChanged); connect(ui->startDateEdit, &KPIM::KDateEdit::dateEntered, this, &EditorView::onStartEditEntered); connect(ui->dueDateEdit, &KPIM::KDateEdit::dateEntered, this, &EditorView::onDueEditEntered); @@ -135,6 +141,26 @@ connect(this, SIGNAL(doneChanged(bool)), m_model, SLOT(setDone(bool))); } +bool EditorView::eventFilter(QObject *watched, QEvent *event) +{ + Q_UNUSED(watched); + switch (event->type()) { + case QEvent::FocusIn: + // We don't want to replace text being edited by the user with older text + // coming from akonadi notifications (async, after some older save job) + m_model->setProperty("reactToNotifications", false); + break; + case QEvent::FocusOut: + // We do react to notifications, however, when not having the focus, + // for instance when changing the title using the central list. + m_model->setProperty("reactToNotifications", true); + break; + default: + break; + } + return false; +} + void EditorView::onArtifactChanged() { auto artifact = m_model->property("artifact").value(); @@ -151,10 +177,10 @@ { const auto title = m_model->property("title").toString(); const auto text = m_model->property("text").toString(); + const auto fullText = title + '\n' + text; - QRegExp reg("^" + QRegExp::escape(title) + "\\s*\\n?" + QRegExp::escape(text) + "\\s*$"); - if (!reg.exactMatch(ui->textEdit->toPlainText())) - ui->textEdit->setPlainText(title + '\n' + text); + if (ui->textEdit->toPlainText() != fullText) // QPlainTextEdit doesn't do this check + ui->textEdit->setPlainText(fullText); } void EditorView::onStartDateChanged() diff --git a/tests/features/cuke-steps.cpp b/tests/features/cuke-steps.cpp --- a/tests/features/cuke-steps.cpp +++ b/tests/features/cuke-steps.cpp @@ -631,12 +631,14 @@ VERIFY(!property.isEmpty()); ScenarioScope context; + VERIFY(context->editor->setProperty("editingInProgress", true)); VERIFY(context->editor->setProperty(property, value)); } WHEN("^I rename the item to \"(.+)\"$") { REGEX_PARAM(QString, title); ScenarioScope context; + VERIFY(context->editor->setProperty("editingInProgress", false)); VERIFY(context->model()->setData(context->index, title, Qt::EditRole)); context->waitForStableState(); } diff --git a/tests/features/zanshin/features/editing/editing-task.feature b/tests/features/zanshin/features/editing/editing-task.feature --- a/tests/features/zanshin/features/editing/editing-task.feature +++ b/tests/features/zanshin/features/editing/editing-task.feature @@ -39,7 +39,8 @@ Scenario: Restoring the task title using the central list Given I display the "Inbox" page And there is an item named "Buy a better book" in the central list - When I rename the item to "Buy a book" + When I open the item in the editor + And I rename the item to "Buy a book" Then the editor shows "Buy a book" as title Scenario: Editing a task start date diff --git a/tests/units/presentation/artifacteditormodeltest.cpp b/tests/units/presentation/artifacteditormodeltest.cpp --- a/tests/units/presentation/artifacteditormodeltest.cpp +++ b/tests/units/presentation/artifacteditormodeltest.cpp @@ -242,6 +242,33 @@ QCOMPARE(model.property(propertyName), propertyValue); } + void shouldNotReactToArtifactPropertyChangesWhenEditing_data() + { + shouldReactToArtifactPropertyChanges_data(); + } + + void shouldNotReactToArtifactPropertyChangesWhenEditing() + { + // GIVEN + QFETCH(Domain::Artifact::Ptr, artifact); + QFETCH(QByteArray, propertyName); + QFETCH(QVariant, propertyValue); + QFETCH(QByteArray, signal); + + Presentation::ArtifactEditorModel model; + model.setArtifact(artifact); + QSignalSpy spy(&model, signal.constData()); + + // WHEN + const auto oldPropertyValue = artifact->property(propertyName); + model.setEditingInProgress(true); + artifact->setProperty(propertyName, propertyValue); + + // THEN + QVERIFY(spy.isEmpty()); + QCOMPARE(model.property(propertyName), oldPropertyValue); + } + void shouldReactToTaskDelegateChanges() { // GIVEN diff --git a/tests/units/widgets/editorviewtest.cpp b/tests/units/widgets/editorviewtest.cpp --- a/tests/units/widgets/editorviewtest.cpp +++ b/tests/units/widgets/editorviewtest.cpp @@ -39,10 +39,17 @@ { Q_OBJECT public: + EditorModelStub() + { + setProperty("reactToNotifications", true); + } + void setPropertyAndSignal(const QByteArray &name, const QVariant &value) { if (property(name) == value) return; + if (!property("reactToNotifications").toBool()) + return; setProperty(name, value); if (name == "artifact") @@ -327,7 +334,7 @@ QCOMPARE(doneButton->isChecked(), model.property("done").toBool()); } - void shouldReactToTitleChanges() + void shouldNotReactToChangesWhileEditing() { // GIVEN Widgets::EditorView editor; @@ -341,23 +348,47 @@ editor.setModel(&model); auto textEdit = editor.findChild(QStringLiteral("textEdit")); + auto startDateEdit = editor.findChild(QStringLiteral("startDateEdit")); + auto dueDateEdit = editor.findChild(QStringLiteral("dueDateEdit")); + auto doneButton = editor.findChild(QStringLiteral("doneButton")); + auto delegateLabel = editor.findChild(QStringLiteral("delegateLabel")); + auto delegateEdit = editor.findChild(QStringLiteral("delegateEdit")); + model.setDelegateText(QStringLiteral("John Doe")); + editor.setModel(&model); // WHEN - model.setPropertyAndSignal("title", "New title"); + editor.show(); + QTest::qWaitForWindowShown(&editor); + editor.activateWindow(); + textEdit->setFocus(); + model.setTitle("New title"); + model.setText("New text"); + startDateEdit->setFocus(); + model.setStartDate(QDateTime::currentDateTime().addDays(1)); + dueDateEdit->setFocus(); + model.setDueDate(QDateTime::currentDateTime().addDays(3)); + doneButton->setFocus(); + model.setDone(false); + delegateEdit->setFocus(); + model.setDelegateText(QStringLiteral("John Smith")); + + // THEN (nothing changed) + QCOMPARE(textEdit->toPlainText(), QStringLiteral("My title\n\nMy text")); + QCOMPARE(startDateEdit->date(), QDate::currentDate()); + QCOMPARE(dueDateEdit->date(), QDate::currentDate().addDays(2)); + QVERIFY(doneButton->isChecked()); + auto expectedText = tr("Delegated to: %1").arg(QStringLiteral("John Doe")); + QCOMPARE(delegateLabel->text(), expectedText); - // THEN - QCOMPARE(textEdit->toPlainText(), QString(model.property("title").toString() - + "\n" - + model.property("text").toString())); } - void shouldNotReactToTitleTrim() + void shouldReactToTitleChanges() { // GIVEN Widgets::EditorView editor; EditorModelStub model; model.makeTaskAvailable(); - model.setProperty("title", "My title "); + model.setProperty("title", "My title"); model.setProperty("text", "\nMy text"); model.setProperty("startDate", QDateTime::currentDateTime()); model.setProperty("dueDate", QDateTime::currentDateTime().addDays(2)); @@ -367,10 +398,10 @@ auto textEdit = editor.findChild(QStringLiteral("textEdit")); // WHEN - model.setPropertyAndSignal("title", "My title"); + model.setPropertyAndSignal("title", "New title"); // THEN - QCOMPARE(textEdit->toPlainText(), QString(model.property("title").toString() + " " + QCOMPARE(textEdit->toPlainText(), QString(model.property("title").toString() + "\n" + model.property("text").toString())); } @@ -399,44 +430,6 @@ + model.property("text").toString())); } - void shouldNotReactToTextTrim_data() - { - QTest::addColumn("title"); - QTest::addColumn("text"); - - QTest::newRow("nominal case") << "My title" << "\nMy text \n "; - QTest::newRow("parentheses in the title") << "My (special) title" << "\nMy text \n "; - QTest::newRow("parentheses in the text") << "My title" << "\nMy (special) text \n "; - QTest::newRow("special char in the title") << "My +title" << "\nMy text \n "; - QTest::newRow("special char in the text") << "My title" << "\nMy +text \n "; - } - - void shouldNotReactToTextTrim() - { - // GIVEN - QFETCH(QString, title); - QFETCH(QString, text); - - Widgets::EditorView editor; - EditorModelStub model; - model.makeTaskAvailable(); - model.setProperty("title", title); - model.setProperty("text", text); - model.setProperty("startDate", QDateTime::currentDateTime()); - model.setProperty("dueDate", QDateTime::currentDateTime().addDays(2)); - model.setProperty("done", true); - editor.setModel(&model); - - auto textEdit = editor.findChild(QStringLiteral("textEdit")); - - // WHEN - model.setPropertyAndSignal("text", text.trimmed()); - - // THEN - QCOMPARE(model.property("text").toString(), text.trimmed()); - QCOMPARE(textEdit->toPlainText(), QString(model.property("title").toString() + "\n" + text)); - } - void shouldApplyTextEditChanges_data() { QTest::addColumn("plainText");