From 0f9e20aa2d6b4af6d9965344c0322c33eced904c Mon Sep 17 00:00:00 2001 From: pascal Date: Tue, 25 Aug 2026 21:49:01 +0200 Subject: [PATCH 01/12] replace JSON with sqlite --- .clang-tidy | 147 +++++++++ .gitignore | 1 + CMakeLists.txt | 3 +- README.md | 46 ++- benchmarks/CMakeLists.txt | 9 + benchmarks/addintervalbenchmark.cpp | 40 +++ benchmarks/createlargeproject.cpp | 64 ++++ benchmarks/createlargeproject.h | 5 + conanfile.py | 1 - src/CMakeLists.txt | 10 +- src/application.cpp | 109 ++++++- src/application.h | 51 ++- src/commands/commands.cpp | 26 ++ src/commands/commands.h | 20 ++ src/commands/undostack.cpp | 98 +++++- src/commands/undostack.h | 46 ++- src/db/CMakeLists.txt | 15 + src/db/abstracttimesheetrepository.cpp | 48 +++ src/db/abstracttimesheetrepository.h | 52 ++++ src/db/database.cpp | 282 +++++++++++++++++ src/db/database.h | 109 +++++++ src/db/entityid.h | 46 +++ src/db/enumnames.cpp | 88 ++++++ src/db/enumnames.h | 23 ++ src/db/migrations.cpp | 96 ++++++ src/db/migrations.h | 26 ++ src/db/sqltimesheetrepository.cpp | 278 +++++++++++++++++ src/db/sqltimesheetrepository.h | 54 ++++ src/db/sqlvalue.cpp | 95 ++++++ src/db/sqlvalue.h | 34 ++ src/enum.h | 15 - src/exceptions.h | 13 +- src/interval.cpp | 13 +- src/interval.h | 27 +- src/intervalmodel.cpp | 43 ++- src/intervalmodel.h | 19 +- src/json.cpp | 61 ---- src/json.h | 39 --- .../kdsingleapplication_lib.h | 23 ++ src/main.cpp | 51 ++- src/mainwindow.cpp | 190 +++++------- src/mainwindow.h | 34 +- src/mainwindow.ui | 34 +- src/period.cpp | 36 --- src/period.h | 6 - src/plan.cpp | 181 ++++++----- src/plan.h | 74 ++++- src/plansettingsdialog.cpp | 65 ++++ src/plansettingsdialog.h | 31 ++ src/plantableview.cpp | 30 +- src/project.cpp | 32 +- src/project.h | 13 +- src/projectmodel.cpp | 20 +- src/projectmodel.h | 7 +- src/serialization.cpp | 127 -------- src/serialization.h | 7 - src/timesheet.cpp | 17 +- src/timesheet.h | 10 + test/unit/CMakeLists.txt | 5 + test/unit/databasetest.cpp | 234 ++++++++++++++ test/unit/enumnamestest.cpp | 49 +++ test/unit/plantest.cpp | 61 ++++ test/unit/qtfixture.h | 28 ++ test/unit/sqlvaluetest.cpp | 72 +++++ test/unit/timesheetrepositorytest.cpp | 290 ++++++++++++++++++ test/unit/undostacktransactiontest.cpp | 202 ++++++++++++ 66 files changed, 3409 insertions(+), 672 deletions(-) create mode 100644 .clang-tidy create mode 100644 benchmarks/CMakeLists.txt create mode 100644 benchmarks/addintervalbenchmark.cpp create mode 100644 benchmarks/createlargeproject.cpp create mode 100644 benchmarks/createlargeproject.h create mode 100644 src/db/CMakeLists.txt create mode 100644 src/db/abstracttimesheetrepository.cpp create mode 100644 src/db/abstracttimesheetrepository.h create mode 100644 src/db/database.cpp create mode 100644 src/db/database.h create mode 100644 src/db/entityid.h create mode 100644 src/db/enumnames.cpp create mode 100644 src/db/enumnames.h create mode 100644 src/db/migrations.cpp create mode 100644 src/db/migrations.h create mode 100644 src/db/sqltimesheetrepository.cpp create mode 100644 src/db/sqltimesheetrepository.h create mode 100644 src/db/sqlvalue.cpp create mode 100644 src/db/sqlvalue.h delete mode 100644 src/enum.h delete mode 100644 src/json.cpp delete mode 100644 src/json.h create mode 100644 src/kdsingleapplication/kdsingleapplication_lib.h create mode 100644 src/plansettingsdialog.cpp create mode 100644 src/plansettingsdialog.h delete mode 100644 src/serialization.cpp delete mode 100644 src/serialization.h create mode 100644 test/unit/databasetest.cpp create mode 100644 test/unit/enumnamestest.cpp create mode 100644 test/unit/qtfixture.h create mode 100644 test/unit/sqlvaluetest.cpp create mode 100644 test/unit/timesheetrepositorytest.cpp create mode 100644 test/unit/undostacktransactiontest.cpp diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000..fd8c681 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,147 @@ +# Generated from CLion Inspection settings +--- +Checks: '-*, +bugprone-argument-comment, +bugprone-assert-side-effect, +bugprone-bad-signal-to-kill-thread, +bugprone-branch-clone, +bugprone-copy-constructor-init, +bugprone-dangling-handle, +bugprone-dynamic-static-initializers, +bugprone-fold-init-type, +bugprone-forward-declaration-namespace, +bugprone-forwarding-reference-overload, +bugprone-inaccurate-erase, +bugprone-incorrect-roundings, +bugprone-integer-division, +bugprone-lambda-function-name, +bugprone-macro-parentheses, +bugprone-macro-repeated-side-effects, +bugprone-misplaced-operator-in-strlen-in-alloc, +bugprone-misplaced-pointer-arithmetic-in-alloc, +bugprone-misplaced-widening-cast, +bugprone-move-forwarding-reference, +bugprone-multiple-statement-macro, +bugprone-no-escape, +bugprone-parent-virtual-call, +bugprone-posix-return, +bugprone-reserved-identifier, +bugprone-sizeof-container, +bugprone-sizeof-expression, +bugprone-spuriously-wake-up-functions, +bugprone-string-constructor, +bugprone-string-integer-assignment, +bugprone-string-literal-with-embedded-nul, +bugprone-suspicious-enum-usage, +bugprone-suspicious-include, +bugprone-suspicious-memset-usage, +bugprone-suspicious-missing-comma, +bugprone-suspicious-semicolon, +bugprone-suspicious-string-compare, +bugprone-suspicious-memory-comparison, +bugprone-suspicious-realloc-usage, +bugprone-swapped-arguments, +bugprone-terminating-continue, +bugprone-throw-keyword-missing, +bugprone-too-small-loop-variable, +bugprone-undefined-memory-manipulation, +bugprone-undelegated-constructor, +bugprone-unhandled-self-assignment, +bugprone-unused-raii, +bugprone-unused-return-value, +bugprone-use-after-move, +bugprone-virtual-near-miss, +cert-dcl21-cpp, +cert-dcl58-cpp, +cert-err34-c, +cert-err52-cpp, +cert-err60-cpp, +cert-flp30-c, +cert-msc50-cpp, +cert-msc51-cpp, +cert-str34-c, +cppcoreguidelines-interfaces-global-init, +cppcoreguidelines-narrowing-conversions, +cppcoreguidelines-pro-type-member-init, +cppcoreguidelines-pro-type-static-cast-downcast, +cppcoreguidelines-slicing, +google-default-arguments, +google-explicit-constructor, +google-runtime-operator, +hicpp-exception-baseclass, +hicpp-multiway-paths-covered, +misc-misplaced-const, +misc-new-delete-overloads, +misc-no-recursion, +misc-non-copyable-objects, +misc-throw-by-value-catch-by-reference, +misc-unconventional-assign-operator, +misc-uniqueptr-reset-release, +modernize-avoid-bind, +modernize-concat-nested-namespaces, +modernize-deprecated-headers, +modernize-deprecated-ios-base-aliases, +modernize-loop-convert, +modernize-make-shared, +modernize-make-unique, +modernize-pass-by-value, +modernize-raw-string-literal, +modernize-redundant-void-arg, +modernize-replace-auto-ptr, +modernize-replace-disallow-copy-and-assign-macro, +modernize-replace-random-shuffle, +modernize-return-braced-init-list, +modernize-shrink-to-fit, +modernize-unary-static-assert, +modernize-use-auto, +modernize-use-bool-literals, +modernize-use-emplace, +modernize-use-equals-default, +modernize-use-equals-delete, +modernize-use-nodiscard, +modernize-use-noexcept, +modernize-use-nullptr, +modernize-use-override, +modernize-use-transparent-functors, +modernize-use-uncaught-exceptions, +mpi-buffer-deref, +mpi-type-mismatch, +openmp-use-default-none, +performance-faster-string-find, +performance-for-range-copy, +performance-implicit-conversion-in-loop, +performance-inefficient-algorithm, +performance-inefficient-string-concatenation, +performance-inefficient-vector-operation, +performance-move-const-arg, +performance-move-constructor-init, +performance-no-automatic-move, +performance-noexcept-move-constructor, +performance-trivially-destructible, +performance-type-promotion-in-math-fn, +performance-unnecessary-copy-initialization, +performance-unnecessary-value-param, +portability-simd-intrinsics, +readability-avoid-const-params-in-decls, +readability-const-return-type, +readability-container-size-empty, +readability-convert-member-functions-to-static, +readability-delete-null-pointer, +readability-deleted-default, +readability-inconsistent-declaration-parameter-name, +readability-make-member-function-const, +readability-misleading-indentation, +readability-misplaced-array-index, +readability-non-const-parameter, +readability-redundant-control-flow, +readability-redundant-declaration, +readability-redundant-function-ptr-dereference, +readability-redundant-smartptr-get, +readability-redundant-string-cstr, +readability-redundant-string-init, +readability-simplify-subscript-expr, +readability-static-accessed-through-instance, +readability-static-definition-in-anonymous-namespace, +readability-string-compare, +readability-uniqueptr-delete-release, +readability-use-anyofallof' \ No newline at end of file diff --git a/.gitignore b/.gitignore index d2ad9ef..4d188a6 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ __pycache__/* .idea/* +build/ diff --git a/CMakeLists.txt b/CMakeLists.txt index ae30a1a..25e4c4d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2,8 +2,7 @@ cmake_minimum_required(VERSION 3.18) project(tire) -find_package(Qt6 6.4 REQUIRED Core Widgets Network) -find_package(nlohmann_json REQUIRED) +find_package(Qt6 6.4 REQUIRED Core Widgets Network Sql) find_package(spdlog REQUIRED) find_package(fmt REQUIRED) diff --git a/README.md b/README.md index 692bd38..656050c 100644 --- a/README.md +++ b/README.md @@ -11,9 +11,9 @@ With a good GUI, it's easy to spot and fix unfinished or ridiculous intervals (l ## Features -- JSON files: Easy to modify manually if required. +- SQLite database: every edit is saved immediately, and your times are queryable with SQL. - GUI that is optimized for speedy edits (many times a day). -- Fast start, load, save. +- Fast start; no save step at all. - Auto-assigned project colors for better overview. - Gantt chart makes it easy to detect mistakes. - Multiple views: Daily, Weekly, Monthly, Yearly. @@ -24,3 +24,45 @@ With a good GUI, it's easy to spot and fix unfinished or ridiculous intervals (l - Full undo/redo support. - Convenience commands like "split interval". - Works with light and dark themes thanks to smart color selection. + +## Storage + +TiRe keeps everything in a SQLite database at `~/.local/share/tire/tire.db` +(`QStandardPaths::AppDataLocation`; the exact directory is platform-specific). It is opened at +startup and written to as you edit, inside a transaction per undo step — so there is no Save +action, and nothing is lost if the app or the machine dies mid-edit. + +Point the app at a different database with `--database`, which is handy for trying things out +without touching your real records: + +```sh +tire --database /tmp/scratch.db +``` + +Only one instance per database runs at a time; starting a second one with the same database +raises the existing window instead. + +### Querying + +The schema is plain SQL, so you can report on it directly: + +```sh +sqlite3 ~/.local/share/tire/tire.db " + SELECT p.name, SUM((julianday(i.end_time) - julianday(i.begin_time)) * 24) AS hours + FROM interval i JOIN project p ON p.id = i.project_id + WHERE i.end_time IS NOT NULL AND i.begin_time >= '2026-01-01' + GROUP BY p.name ORDER BY hours DESC;" +``` + +An interval with `end_time IS NULL` is one that is still running. + +### Backups + +Use SQLite's own backup command rather than copying the file: + +```sh +sqlite3 ~/.local/share/tire/tire.db ".backup /path/to/backup.db" +``` + +The database runs in WAL mode, so recent changes may live in the `tire.db-wal` sidecar file. +Copying `tire.db` on its own while the app is running will silently miss them. diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt new file mode 100644 index 0000000..6563716 --- /dev/null +++ b/benchmarks/CMakeLists.txt @@ -0,0 +1,9 @@ +add_executable(add-interval-benchmark addintervalbenchmark.cpp) +add_library(tire-benchmark-utils STATIC + createlargeproject.cpp + createlargeproject.h +) +target_compile_features(add-interval-benchmark PUBLIC cxx_std_20) +target_compile_features(tire-benchmark-utils PUBLIC cxx_std_20) +target_link_libraries(tire-benchmark-utils PUBLIC tire-impl) +target_link_libraries(add-interval-benchmark PUBLIC tire-impl tire-benchmark-utils) diff --git a/benchmarks/addintervalbenchmark.cpp b/benchmarks/addintervalbenchmark.cpp new file mode 100644 index 0000000..bdbf562 --- /dev/null +++ b/benchmarks/addintervalbenchmark.cpp @@ -0,0 +1,40 @@ +#include "createlargeproject.h" +#include "intervalmodel.h" +#include "plan.h" +#include "projectmodel.h" +#include "timesheet.h" + +class RAIITimer +{ +public: + explicit RAIITimer(std::string name) : m_name(std::move(name)), m_start(clock::now()) + { + } + + ~RAIITimer() + { + const auto end = clock::now(); + using std::chrono_literals::operator""ms; + fmt::print("{}: {:< 8.4}ms\n", m_name, (end - m_start) / 1.0ms); + } + + RAIITimer(const RAIITimer& other) = delete; + RAIITimer(RAIITimer&& other) = delete; + RAIITimer& operator=(const RAIITimer& other) = delete; + RAIITimer& operator=(RAIITimer&& other) = delete; + + using clock = std::chrono::steady_clock; + +private: + const std::string m_name; + const std::chrono::time_point m_start; +}; + +int main() +{ + for (const int exponent : std::views::iota(0, 20)) { + const auto n = std::pow(2, exponent); + RAIITimer timer(fmt::format("n = {:8}", n)); + const auto time_sheet = ::create_large_time_sheet(n); + } +} diff --git a/benchmarks/createlargeproject.cpp b/benchmarks/createlargeproject.cpp new file mode 100644 index 0000000..5627735 --- /dev/null +++ b/benchmarks/createlargeproject.cpp @@ -0,0 +1,64 @@ +#include "intervalmodel.h" +#include "plan.h" +#include "projectmodel.h" +#include "timesheet.h" + +#include + +template class TimeAdvancer +{ +public: + explicit TimeAdvancer(RNG& rng) : m_rng(rng) + { + } + + QDateTime next() + { + using std::chrono_literals::operator""min; + using std::chrono_literals::operator""h; + static constexpr auto min_advance = 1min; + static constexpr auto max_advance = 24h; + static std::uniform_int_distribution minute_distribution(min_advance / 1min, max_advance / 1min); + + using std::chrono_literals::operator""s; + const auto ret = current_timestamp; + current_timestamp = current_timestamp.addSecs(minute_distribution(m_rng) * 60); + return ret; + } + +private: + RNG& m_rng; + + // current_timestamp needs to be initialized with an arbitrary date-time. + // It's important that it doesn't change to ensure reproducible results. + QDateTime current_timestamp = {QDate{2025, 1, 16}, QTime{21, 02, 45}}; +}; + +TimeSheet create_large_time_sheet(const int interval_count) +{ + TimeSheet ts; + const auto projects = std::vector{ + &ts.project_model().add(std::make_unique("P1", Qt::red)), + &ts.project_model().add(std::make_unique("P2", Qt::green)), + &ts.project_model().add(std::make_unique("P3", Qt::blue)), + &ts.project_model().add(std::make_unique("P4", Qt::yellow)), + &ts.project_model().add(std::make_unique("P5", Qt::magenta)), + &ts.project_model().add(std::make_unique("P6", Qt::cyan)), + }; + + // Use a constant seed to ensure reproducible results. + static constexpr auto seed = 42; + std::mt19937 rng{seed}; + std::uniform_int_distribution project_distribution(0, static_cast(projects.size()) - 1); + + auto time_advancer = TimeAdvancer{rng}; + + for (int i = 0; i < interval_count; ++i) { + const auto* const project = projects.at(project_distribution(rng)); + auto interval = std::make_unique(project); + interval->swap_begin(time_advancer.next()); + interval->swap_begin(time_advancer.next()); + ts.interval_model().add(std::move(interval)); + } + return ts; +} diff --git a/benchmarks/createlargeproject.h b/benchmarks/createlargeproject.h new file mode 100644 index 0000000..45c7ea6 --- /dev/null +++ b/benchmarks/createlargeproject.h @@ -0,0 +1,5 @@ +#pragma once + +class TimeSheet; + +[[nodiscard]] TimeSheet create_large_time_sheet(int interval_count); diff --git a/conanfile.py b/conanfile.py index ecf23a7..394e2cd 100644 --- a/conanfile.py +++ b/conanfile.py @@ -33,7 +33,6 @@ def requirements(self): "shared": True, }) self.requires("spdlog/[>=1.14]") - self.requires("nlohmann_json/[>=3.0]") self.requires("fmt/[>=10.0]") def config_options(self): diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 56d30ac..7f73178 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -4,7 +4,6 @@ add_library(tire-impl STATIC) target_sources(tire-impl PRIVATE application.cpp application.h - enum.h enumcombobox.h exceptions.h fmt.h @@ -14,8 +13,6 @@ target_sources(tire-impl PRIVATE interval.h intervalmodel.cpp intervalmodel.h - json.cpp - json.h mainwindow.cpp mainwindow.h mainwindow.ui @@ -26,14 +23,14 @@ target_sources(tire-impl PRIVATE periodedit.ui plan.cpp plan.h + plansettingsdialog.cpp + plansettingsdialog.h plantableview.cpp plantableview.h project.cpp project.h projectmodel.cpp projectmodel.h - serialization.cpp - serialization.h tableview.cpp tableview.h timesheet.cpp @@ -57,12 +54,13 @@ target_sources(tire-impl PRIVATE splitpointeditor.ui ) -target_link_libraries(tire-impl PUBLIC Qt6::Core Qt6::Widgets Qt6::Network spdlog::spdlog fmt::fmt nlohmann_json::nlohmann_json) +target_link_libraries(tire-impl PUBLIC Qt6::Core Qt6::Widgets Qt6::Network Qt6::Sql spdlog::spdlog fmt::fmt) target_link_libraries(tire PRIVATE tire-impl) target_compile_features(tire PUBLIC cxx_std_20) target_compile_features(tire-impl PUBLIC cxx_std_20) target_include_directories(tire-impl PUBLIC ${CMAKE_SOURCE_DIR}/src) add_subdirectory(commands) +add_subdirectory(db) add_subdirectory(views) add_subdirectory(kdsingleapplication) diff --git a/src/application.cpp b/src/application.cpp index 7f407b4..2bcd1dd 100644 --- a/src/application.cpp +++ b/src/application.cpp @@ -1,53 +1,97 @@ #include "application.h" #include "commands/undostack.h" +#include "db/database.h" +#include "db/sqltimesheetrepository.h" +#include "exceptions.h" #include "fmt.h" #include #include +#include #include +#include +#include +#include std::optional Application::m_current_date_time = std::nullopt; -std::filesystem::path Application::m_timesheet_filename = {}; +std::filesystem::path Application::m_database_path = {}; +bool Application::m_is_default_database = true; std::unique_ptr Application::m_undo_stack = std::make_unique(); +Database* Application::m_database_instance = nullptr; +SqlTimeSheetRepository* Application::m_repository_instance = nullptr; namespace { -constexpr auto timesheet_filename_option_name = "timesheet-filename"; +constexpr auto database_option_name = "database"; constexpr auto current_date_time_option_name = "current-date-time"; +constexpr auto database_file_name = "tire.db"; [[nodiscard]] auto command_line_args() { auto clp = std::make_unique(); - clp->addPositionalArgument(timesheet_filename_option_name, "Path to the time sheet (JSON).", "FILENAME"); + clp->addOption(QCommandLineOption{ + database_option_name, + "Use this database instead of the one in the application data directory. Useful for testing.", "PATH"}); clp->addOption(QCommandLineOption{ current_date_time_option_name, "Fix the current date time to this value (ISO format). Useful for reproducible debugging and testing.", "CURRENT_DATE_TIME"}); + clp->addHelpOption(); + clp->addVersionOption(); clp->process(*QApplication::instance()); return clp; } +[[nodiscard]] std::filesystem::path default_database_path() +{ + // Requires setApplicationName/setOrganizationName to have run already. + const auto directory = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation); + return static_cast(QDir{directory}.filePath(database_file_name).toStdString()); +} + } // namespace Application::Application(int& argc, char** argv) : m_qapp(std::make_unique(argc, argv)) { + // Must precede any QStandardPaths query, and fixes the empty caption every message box in the + // application would otherwise show. + // + // The organization name is deliberately left unset: QStandardPaths::AppDataLocation appends + // both organization and application, so setting both would nest the database in + // ~/.local/share/tire/tire/. Nothing here uses QSettings, so the organization buys nothing. + QCoreApplication::setApplicationName(QStringLiteral("tire")); + QGuiApplication::setApplicationDisplayName(QObject::tr("TiRe")); + const auto args = command_line_args(); if (const auto v = args->value(current_date_time_option_name); !v.isEmpty()) { m_current_date_time = QDateTime::fromString(v, Qt::ISODate); if (!m_current_date_time->isValid()) { fmt::println("Value '{}' is not a valid ISO date time format.", v); - QApplication::exit(1); + m_current_date_time = std::nullopt; } else { fmt::println("Simulating today = {}", *m_current_date_time); } } - if (const auto filenames = args->positionalArguments(); !filenames.empty()) { - m_timesheet_filename = static_cast(filenames.front().toStdString()); + + if (const auto v = args->value(database_option_name); !v.isEmpty()) { + m_database_path = static_cast(v.toStdString()); + m_is_default_database = false; + } else { + m_database_path = ::default_database_path(); + m_is_default_database = true; } } -Application::~Application() = default; +Application::~Application() +{ + // Order matters: the repository refers to the database, and the database must be gone before + // m_qapp is destroyed so that the SQL driver plugin is still loaded when the connection closes. + m_repository_instance = nullptr; + m_database_instance = nullptr; + m_repository.reset(); + m_database.reset(); +} QDateTime Application::current_date_time() { @@ -57,9 +101,56 @@ QDateTime Application::current_date_time() return QDateTime::currentDateTime(); } -const std::filesystem::path& Application::timesheet_filename() noexcept +const std::filesystem::path& Application::database_path() noexcept +{ + return m_database_path; +} + +bool Application::is_default_database() noexcept +{ + return m_is_default_database; +} + +QString Application::single_instance_name() +{ + // Hash rather than embed the path: the socket name has a length limit that would truncate a + // long path, and truncation could alias two different databases onto one lock. + const auto canonical = QFileInfo{QString::fromStdString(m_database_path.string())}.absoluteFilePath(); + const auto digest = QCryptographicHash::hash(canonical.toUtf8(), QCryptographicHash::Sha1); + return QStringLiteral("tire-%1").arg(QString::fromLatin1(digest.toHex().left(8))); +} + +Application::DatabaseOpenResult Application::open_database() +{ + const auto path = QString::fromStdString(m_database_path.string()); + const auto directory = QFileInfo{path}.absolutePath(); + if (!QDir{}.mkpath(directory)) { + return {.ok = false, .message = QObject::tr("Cannot create the directory '%1' for the database.").arg(directory)}; + } + + try { + m_database = std::make_unique(Database::open_file(m_database_path)); + m_database->migrate(); + m_repository = std::make_unique(*m_database); + } catch (const DatabaseError& e) { + m_repository.reset(); + m_database.reset(); + return {.ok = false, .message = QString::fromStdString(e.what())}; + } + + m_database_instance = m_database.get(); + m_repository_instance = m_repository.get(); + return {.ok = true, .message = {}}; +} + +Database* Application::database() noexcept +{ + return m_database_instance; +} + +SqlTimeSheetRepository& Application::sql_repository() noexcept { - return m_timesheet_filename; + return *m_repository_instance; } UndoStack& Application::undo_stack() noexcept diff --git a/src/application.h b/src/application.h index d7214b1..f43a4ca 100644 --- a/src/application.h +++ b/src/application.h @@ -1,9 +1,12 @@ #pragma once +#include #include #include #include +class Database; +class SqlTimeSheetRepository; class UndoStack; class QApplication; class QDateTime; @@ -13,15 +16,59 @@ class Application public: explicit Application(int& argc, char** argv); ~Application(); + Application(const Application&) = delete; + Application& operator=(const Application&) = delete; + Application(Application&&) = delete; + Application& operator=(Application&&) = delete; [[nodiscard]] static QDateTime current_date_time(); - [[nodiscard]] static const std::filesystem::path& timesheet_filename() noexcept; [[nodiscard]] static UndoStack& undo_stack() noexcept; QApplication& qapp() const noexcept; + /** @brief Where the timesheet database lives; the --database option overrides the default. */ + [[nodiscard]] static const std::filesystem::path& database_path() noexcept; + + /** @brief Whether database_path() is the standard location rather than an override. */ + [[nodiscard]] static bool is_default_database() noexcept; + + /** + * @brief A name identifying this database for the single-instance lock. + * Derived from the path so that "one process per database" holds, rather than + * "one process for the whole application" -- otherwise a --database override would be ignored + * whenever the regular instance is running. + */ + [[nodiscard]] static QString single_instance_name(); + + struct DatabaseOpenResult + { + bool ok; + QString message; ///< Empty when ok; ready to show to the user otherwise. + }; + + /** + * @brief Opens and migrates the database at database_path(). + * Reports failure rather than throwing, because the caller's only sensible reaction is to show + * the message and exit. + */ + [[nodiscard]] DatabaseOpenResult open_database(); + + /** @pre open_database() returned ok. */ + [[nodiscard]] static SqlTimeSheetRepository& sql_repository() noexcept; + + /** @brief The open database, or nullptr when none was opened (as in tests and benchmarks). */ + [[nodiscard]] static Database* database() noexcept; + private: std::unique_ptr m_qapp; + // Held by value-owning pointers on the instance rather than as file-scope statics: the + // connection must be closed while QApplication is still alive, or Qt unloads the SQL driver + // plugin out from under it. + std::unique_ptr m_database; + std::unique_ptr m_repository; static std::optional m_current_date_time; - static std::filesystem::path m_timesheet_filename; + static std::filesystem::path m_database_path; + static bool m_is_default_database; static std::unique_ptr m_undo_stack; + static Database* m_database_instance; + static SqlTimeSheetRepository* m_repository_instance; }; diff --git a/src/commands/commands.cpp b/src/commands/commands.cpp index 5c37926..c338b57 100644 --- a/src/commands/commands.cpp +++ b/src/commands/commands.cpp @@ -14,6 +14,32 @@ void delete_intervals(IntervalModel& interval_model, const std::set make_modify_plan_kind_command(Plan& plan, const Plan::Entry& entry, const Plan::Kind kind) +{ + const auto swapper = [&entry](Plan& p, const Plan::Kind k) { return p.swap_kind(entry, k); }; + return make_modify_command(plan, kind, swapper, [] {}); +} + +std::unique_ptr make_modify_plan_period_command(Plan& plan, const Plan::Entry& entry, Period period) +{ + const auto swapper = [&entry](Plan& p, Period q) { return p.swap_period(entry, std::move(q)); }; + return make_modify_command(plan, std::move(period), swapper, [] {}); +} + +std::unique_ptr make_modify_plan_start_command(Plan& plan, QDate start) +{ + const auto swapper = [](Plan& p, QDate s) { return p.swap_start(s); }; + return make_modify_command(plan, start, swapper, [] {}); +} + +std::unique_ptr make_modify_plan_overtime_offset_command(Plan& plan, const std::chrono::minutes offset) +{ + const auto swapper = [](Plan& p, const std::chrono::minutes o) { return p.swap_overtime_offset(o); }; + return make_modify_command(plan, offset, swapper, [] {}); +} + void split_interval(IntervalModel& interval_model, const Interval& interval) { SplitPointEditor e; diff --git a/src/commands/commands.h b/src/commands/commands.h index 550ab8b..aade35e 100644 --- a/src/commands/commands.h +++ b/src/commands/commands.h @@ -2,6 +2,7 @@ #include "commands/modifycommand.h" #include "intervalmodel.h" +#include "plan.h" #include #include @@ -13,6 +14,21 @@ class IntervalModel; void delete_intervals(IntervalModel& interval_model, const std::set& selection); void split_interval(IntervalModel& interval_model, const Interval& interval); +/** + * @name Plan edit commands + * Plan::swap_kind and Plan::swap_period already persist and emit, so these carry no extra signal. + * @{ + */ +[[nodiscard]] std::unique_ptr make_modify_plan_kind_command(Plan& plan, const Plan::Entry& entry, + Plan::Kind kind); +/** @pre Plan::can_set_period(entry, period) -- redo() must not throw. */ +[[nodiscard]] std::unique_ptr make_modify_plan_period_command(Plan& plan, const Plan::Entry& entry, + Period period); +[[nodiscard]] std::unique_ptr make_modify_plan_start_command(Plan& plan, QDate start); +[[nodiscard]] std::unique_ptr make_modify_plan_overtime_offset_command(Plan& plan, + std::chrono::minutes offset); +/** @} */ + template std::unique_ptr make_modify_interval_command(IntervalModel& interval_model, IntervalT& interval, Value other_value, Swapper swapper) { @@ -21,6 +37,10 @@ make_modify_interval_command(IntervalModel& interval_model, IntervalT& interval, std::move(swapper)); } else { const auto signal = [&interval_model, &interval]() { + // Persist before notifying, so no observer can see state the database does not have. This + // is the write-through hook for Interval's swap_* setters, which are themselves unaware of + // persistence; it covers undo as well, because ModifyCommand::undo() calls redo(). + interval_model.persist(interval); const auto index = interval_model.index(interval); Q_EMIT interval_model.dataChanged(index, index.siblingAtColumn(interval_model.columnCount({}) - 1)); Q_EMIT interval_model.data_changed(); diff --git a/src/commands/undostack.cpp b/src/commands/undostack.cpp index bec81d1..5cf83ff 100644 --- a/src/commands/undostack.cpp +++ b/src/commands/undostack.cpp @@ -1,5 +1,9 @@ #include "commands/undostack.h" +#include "application.h" #include "commands/command.h" +#include "exceptions.h" + +#include const QUndoStack& UndoStack::impl() const noexcept { @@ -11,22 +15,106 @@ QUndoStack& UndoStack::impl() noexcept return m_impl; } +void UndoStack::run_in_transaction(const std::function& action, const QString& what) +{ + auto* const database = Application::database(); + if (database == nullptr) { + // No database configured, as in tests and benchmarks: just perform the change. + action(); + return; + } + + try { + Database::Transaction transaction{*database}; + action(); + transaction.commit(); + // Only the outermost scope actually commits. Reporting success from an inner one -- inside a + // macro -- would clear a failure indicator before anything had reached the disk. + if (!database->in_transaction()) { + Q_EMIT write_succeeded(); + } + } catch (const DatabaseError& e) { + // The transaction has rolled back, so the database is consistent -- but the in-memory change + // has already happened and is deliberately left in place. A user who can still see the edit + // and is told loudly that it was not saved can copy it out; one whose edit silently vanishes + // from the screen cannot. + // + // Note this must not try to "undo" the failed command to compensate: QUndoStack::push() + // executes redo() *before* appending, so a command whose redo() threw is not on the stack, + // and undoing here would revert the previous, perfectly good command instead. + spdlog::error("{} could not be written: {}", what.toStdString(), e.what()); + Q_EMIT write_failed(QString::fromStdString(e.what())); + } +} + void UndoStack::push(std::unique_ptr command) { - m_impl.push(command.release()); + auto* const raw_command = command.release(); + run_in_transaction( + [this, raw_command] { + try { + m_impl.push(raw_command); + } catch (...) { + // push() executes redo() first and only then takes ownership, so a throwing redo() + // leaves the command with us. + delete raw_command; + throw; + } + }, + QObject::tr("The change")); +} + +void UndoStack::undo() +{ + run_in_transaction([this] { m_impl.undo(); }, QObject::tr("Undoing the change")); +} + +void UndoStack::redo() +{ + run_in_transaction([this] { m_impl.redo(); }, QObject::tr("Redoing the change")); } -UndoStack::Macro::Macro(const QString& text, QUndoStack& stack) : m_stack(stack) +UndoStack::Macro::Macro(const QString& text, UndoStack& stack) : m_stack(stack) { - m_stack.beginMacro(text); + // One transaction spanning the whole macro. The per-push transactions inside collapse into it + // by depth counting, so the compound action commits exactly once. + if (auto* const database = Application::database(); database != nullptr) { + try { + m_transaction = std::make_unique(*database); + } catch (const DatabaseError& e) { + // Every caller is a Qt slot or a dialog's accept(), so letting this escape would unwind + // through the event loop and terminate. Report it like any other write failure instead; the + // individual pushes inside will fail and report on their own too. + spdlog::error("Cannot begin a transaction for '{}': {}", text.toStdString(), e.what()); + Q_EMIT m_stack.write_failed(QString::fromStdString(e.what())); + } + } + m_stack.impl().beginMacro(text); } UndoStack::Macro::~Macro() { - m_stack.endMacro(); + // endMacro first: beginMacro/endMacro only group commands that already ran, so nothing executes + // after the commit. + m_stack.impl().endMacro(); + if (m_transaction == nullptr) { + return; + } + + auto failure = QString{}; + try { + m_transaction->commit(); + } catch (const DatabaseError& e) { + spdlog::error("The compound change could not be written: {}", e.what()); + failure = QString::fromStdString(e.what()); + } + // Destroy the transaction before notifying: the failure handler opens a modal dialog, which + // spins a nested event loop, and that must not happen while an open transaction is still alive. + m_transaction.reset(); + failure.isEmpty() ? Q_EMIT m_stack.write_succeeded() : Q_EMIT m_stack.write_failed(failure); } std::unique_ptr UndoStack::start_macro(const QString& text) { - return std::make_unique(text, m_impl); + return std::make_unique(text, *this); } diff --git a/src/commands/undostack.h b/src/commands/undostack.h index c5191cc..e4d3bdf 100644 --- a/src/commands/undostack.h +++ b/src/commands/undostack.h @@ -1,28 +1,66 @@ #pragma once +#include "db/database.h" + +#include #include +#include class Command; -class UndoStack +/** + * @class UndoStack undostack.h "commands/undostack.h" + * @brief The application's undo stack, and the transaction boundary around every edit. + * + * Each top-level entry -- a push, an undo, a redo, or a whole macro -- runs inside one database + * transaction, so a compound action such as deleting forty intervals either lands completely or + * not at all, and costs one commit rather than forty. + */ +class UndoStack : public QObject { + Q_OBJECT public: [[nodiscard]] const QUndoStack& impl() const noexcept; [[nodiscard]] QUndoStack& impl() noexcept; + void push(std::unique_ptr command); + /** + * @name Undoing and redoing + * These exist so that undo and redo get a transaction too. QUndoStack::undo()/redo() would + * bypass push() entirely, and undoing a macro would then run one transaction per child command. + * @{ + */ + void undo(); + void redo(); + /** @} */ + class Macro { public: - explicit Macro(const QString& text, QUndoStack& stack); + explicit Macro(const QString& text, UndoStack& stack); ~Macro(); + Macro(const Macro&) = delete; + Macro& operator=(const Macro&) = delete; + Macro(Macro&&) = delete; + Macro& operator=(Macro&&) = delete; private: - QUndoStack& m_stack; + UndoStack& m_stack; + std::unique_ptr m_transaction; }; - std::unique_ptr start_macro(const QString& text); + [[nodiscard]] std::unique_ptr start_macro(const QString& text); + +Q_SIGNALS: + /** @brief An edit could not be written. The in-memory state is ahead of the database. */ + void write_failed(const QString& message); + /** @brief An edit was written successfully; any previous failure is over. */ + void write_succeeded(); private: + /** @brief Runs @p action inside a transaction, reporting rather than propagating failures. */ + void run_in_transaction(const std::function& action, const QString& what); + QUndoStack m_impl; }; diff --git a/src/db/CMakeLists.txt b/src/db/CMakeLists.txt new file mode 100644 index 0000000..cc88ee9 --- /dev/null +++ b/src/db/CMakeLists.txt @@ -0,0 +1,15 @@ +target_sources(tire-impl PRIVATE + abstracttimesheetrepository.cpp + abstracttimesheetrepository.h + database.cpp + database.h + entityid.h + enumnames.cpp + enumnames.h + migrations.cpp + migrations.h + sqltimesheetrepository.cpp + sqltimesheetrepository.h + sqlvalue.cpp + sqlvalue.h +) diff --git a/src/db/abstracttimesheetrepository.cpp b/src/db/abstracttimesheetrepository.cpp new file mode 100644 index 0000000..be0f5d4 --- /dev/null +++ b/src/db/abstracttimesheetrepository.cpp @@ -0,0 +1,48 @@ +#include "db/abstracttimesheetrepository.h" + +namespace +{ + +class NullTimeSheetRepository final : public AbstractTimeSheetRepository +{ + void insert(Project&) override + { + } + void update(const Project&) override + { + } + void remove(const Project&) override + { + } + void insert(Interval&) override + { + } + void update(const Interval&) override + { + } + void remove(const Interval&) override + { + } + void insert(Plan::Entry&) override + { + } + void update(const Plan::Entry&) override + { + } + void remove(const Plan::Entry&) override + { + } + void update_plan_setting(const Plan&) override + { + } +}; + +} // namespace + +AbstractTimeSheetRepository::~AbstractTimeSheetRepository() = default; + +AbstractTimeSheetRepository& null_repository() noexcept +{ + static NullTimeSheetRepository repository; + return repository; +} diff --git a/src/db/abstracttimesheetrepository.h b/src/db/abstracttimesheetrepository.h new file mode 100644 index 0000000..e24ee3b --- /dev/null +++ b/src/db/abstracttimesheetrepository.h @@ -0,0 +1,52 @@ +#pragma once + +#include "plan.h" + +class Interval; +class Project; + +/** + * @class AbstractTimeSheetRepository abstracttimesheetrepository.h "db/abstracttimesheetrepository.h" + * @brief Persists individual entities as they change. + * + * The models call into this on every mutation, so the store is always up to date and there is + * nothing to save. Only the models talk to a repository -- leaf types such as Project and + * Interval stay unaware of persistence. + * + * insert() takes a non-const reference because it assigns the entity's id when it does not have + * one yet. An entity that already carries an id keeps it, which is what lets a removal undone by + * a command restore the original row. + */ +class AbstractTimeSheetRepository +{ +public: + AbstractTimeSheetRepository() = default; + virtual ~AbstractTimeSheetRepository(); + AbstractTimeSheetRepository(const AbstractTimeSheetRepository&) = delete; + AbstractTimeSheetRepository& operator=(const AbstractTimeSheetRepository&) = delete; + AbstractTimeSheetRepository(AbstractTimeSheetRepository&&) = delete; + AbstractTimeSheetRepository& operator=(AbstractTimeSheetRepository&&) = delete; + + virtual void insert(Project& project) = 0; + virtual void update(const Project& project) = 0; + virtual void remove(const Project& project) = 0; + + virtual void insert(Interval& interval) = 0; + virtual void update(const Interval& interval) = 0; + virtual void remove(const Interval& interval) = 0; + + virtual void insert(Plan::Entry& entry) = 0; + virtual void update(const Plan::Entry& entry) = 0; + virtual void remove(const Plan::Entry& entry) = 0; + + /** @brief Stores the plan's start date and overtime offset (a single row). */ + virtual void update_plan_setting(const Plan& plan) = 0; +}; + +/** + * @brief A repository that discards everything. + * + * This is the default for models constructed without a database, which keeps them usable in + * tests and benchmarks without any SQL in sight. + */ +[[nodiscard]] AbstractTimeSheetRepository& null_repository() noexcept; diff --git a/src/db/database.cpp b/src/db/database.cpp new file mode 100644 index 0000000..4cc6909 --- /dev/null +++ b/src/db/database.cpp @@ -0,0 +1,282 @@ +#include "db/database.h" + +#include "db/migrations.h" +#include "exceptions.h" + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace +{ + +constexpr auto sqlite_driver = "QSQLITE"; +constexpr auto in_memory_path = ":memory:"; + +[[nodiscard]] QString make_connection_name() +{ + return QStringLiteral("tire-%1").arg(QUuid::createUuid().toString(QUuid::WithoutBraces)); +} + +[[nodiscard]] std::string error_text(const QSqlError& error) +{ + return error.text().trimmed().toStdString(); +} + +} // namespace + +Database::Database(QString connection_name, const Dialect dialect) + : m_connection_name(std::move(connection_name)), m_dialect(dialect), m_thread(QThread::currentThread()) +{ +} + +// Moving is only meaningful for a Database with no open transaction: a live Transaction holds a +// reference to its Database, so moving out from under one would leave it decrementing a counter +// that no longer belongs to the connection. Both operations therefore leave the source fully +// neutralized, so its destructor does nothing. +Database::Database(Database&& other) noexcept + : m_connection_name(std::move(other.m_connection_name)) + , m_dialect(other.m_dialect) + , m_thread(other.m_thread) + , m_transaction_depth(other.m_transaction_depth) + , m_transaction_poisoned(other.m_transaction_poisoned) +{ + Q_ASSERT(m_transaction_depth == 0); + other.m_connection_name.clear(); + other.m_transaction_depth = 0; + other.m_transaction_poisoned = false; + other.m_thread = nullptr; +} + +Database& Database::operator=(Database&& other) noexcept +{ + if (this != &other) { + Q_ASSERT(m_transaction_depth == 0 && other.m_transaction_depth == 0); + if (!m_connection_name.isEmpty()) { + QSqlDatabase::removeDatabase(m_connection_name); + } + m_connection_name = std::move(other.m_connection_name); + m_dialect = other.m_dialect; + m_thread = other.m_thread; + m_transaction_depth = other.m_transaction_depth; + m_transaction_poisoned = other.m_transaction_poisoned; + other.m_connection_name.clear(); + other.m_transaction_depth = 0; + other.m_transaction_poisoned = false; + other.m_thread = nullptr; + } + return *this; +} + +Database::~Database() +{ + if (m_connection_name.isEmpty()) { + return; + } + { + // The handle must go out of scope before removeDatabase, or Qt warns that the connection is + // still in use and keeps it alive. + auto database = QSqlDatabase::database(m_connection_name, false); + if (database.isOpen() && m_transaction_depth > 0) { + spdlog::error("Closing the database with an unfinished transaction; rolling it back."); + database.rollback(); + } + } + QSqlDatabase::removeDatabase(m_connection_name); +} + +Database Database::open_file(const std::filesystem::path& path) +{ + if (!QSqlDatabase::isDriverAvailable(sqlite_driver)) { + throw DatabaseError("The {} database driver is not available. The Qt SQL drivers are plugins; " + "this build of Qt appears to ship without the SQLite one.", + sqlite_driver); + } + + auto connection_name = make_connection_name(); + { + auto database = QSqlDatabase::addDatabase(sqlite_driver, connection_name); + database.setDatabaseName(QString::fromStdString(path.string())); + if (!database.open()) { + const auto message = error_text(database.lastError()); + database = {}; + QSqlDatabase::removeDatabase(connection_name); + throw DatabaseError("Cannot open the database '{}': {}", path.string(), message); + } + } + + Database result{std::move(connection_name), Dialect::SQLITE}; + try { + // SQLite ignores foreign keys unless they are switched on per connection; PostgreSQL always + // enforces them. WAL plus synchronous=NORMAL is the right trade for a desktop app doing one + // small transaction per user action. + result.execute(QStringLiteral("PRAGMA foreign_keys = ON")); + if (path != in_memory_path) { + result.execute(QStringLiteral("PRAGMA journal_mode = WAL")); + result.execute(QStringLiteral("PRAGMA synchronous = NORMAL")); + } + result.execute(QStringLiteral("PRAGMA busy_timeout = 5000")); + } catch (const DatabaseError& e) { + // SQLite defers reading the file header until the first real access, so this is where a file + // that is not a database, or is locked by another process, actually surfaces. Report it in + // terms of the file rather than of the pragma that happened to trip over it. + throw DatabaseError("Cannot use '{}' as a database. It may be corrupt, or in use by another " + "program. Details: {}", + path.string(), e.what()); + } + return result; +} + +Database Database::open_in_memory() +{ + return open_file(in_memory_path); +} + +Database::Dialect Database::dialect() const noexcept +{ + return m_dialect; +} + +bool Database::in_transaction() const noexcept +{ + return m_transaction_depth > 0; +} + +void Database::verify_thread() const +{ + if (QThread::currentThread() != m_thread) { + throw DatabaseError("The database was used from a thread other than the one that opened it. " + "A QSqlDatabase connection may only be used from its creating thread."); + } +} + +QSqlQuery Database::prepare(const QString& statement) const +{ + verify_thread(); + QSqlQuery query{QSqlDatabase::database(m_connection_name, false)}; + if (!query.prepare(statement)) { + throw DatabaseError("Cannot prepare '{}': {}", statement.toStdString(), error_text(query.lastError())); + } + return query; +} + +void Database::execute(QSqlQuery& query) const +{ + verify_thread(); + if (!query.exec()) { + throw DatabaseError("Cannot execute '{}': {}", query.lastQuery().toStdString(), error_text(query.lastError())); + } +} + +void Database::execute(const QString& statement) const +{ + auto query = prepare(statement); + execute(query); +} + +int Database::schema_version() const +{ + // Probing with a plain SELECT keeps this portable: sqlite_master is SQLite-only and + // information_schema is PostgreSQL-only. A missing table simply means "not migrated yet". + try { + auto query = prepare(QStringLiteral("SELECT MAX(version) FROM schema_version")); + execute(query); + if (!query.next() || query.value(0).isNull()) { + return 0; + } + return query.value(0).toInt(); + } catch (const DatabaseError&) { + return 0; + } +} + +void Database::migrate() +{ + migrate_to(latest_schema_version()); +} + +void Database::migrate_to(const int target_version) +{ + const auto current_version = schema_version(); + if (current_version > latest_schema_version()) { + throw DatabaseError("The database has schema version {}, but this build only understands up to {}. " + "Refusing to open it, because an older build could corrupt a newer schema.", + current_version, latest_schema_version()); + } + + for (const auto& migration : migrations()) { + if (migration.version <= current_version || migration.version > target_version) { + continue; + } + spdlog::info("Applying schema migration {}: {}", migration.version, migration.description); + // Each migration commits on its own, so a failure leaves the database at the previous + // version rather than half-migrated. Both SQLite and PostgreSQL have transactional DDL. + Transaction transaction{*this}; + migration.apply(*this); + auto query = prepare(QStringLiteral("INSERT INTO schema_version (version, applied_at) VALUES (?, ?)")); + query.addBindValue(migration.version); + query.addBindValue(QDateTime::currentDateTime().toString(Qt::ISODate)); + execute(query); + transaction.commit(); + } +} + +Database::Transaction::Transaction(Database& database) : m_database(database) +{ + m_database.verify_thread(); + if (m_database.m_transaction_depth == 0) { + m_database.m_transaction_poisoned = false; + if (!QSqlDatabase::database(m_database.m_connection_name, false).transaction()) { + throw DatabaseError("Cannot begin a transaction."); + } + } + ++m_database.m_transaction_depth; +} + +void Database::Transaction::commit() +{ + if (m_committed) { + return; + } + if (m_database.m_transaction_poisoned) { + throw DatabaseError("Cannot commit: an enclosed transaction scope was abandoned."); + } + // The COMMIT happens here rather than in the destructor so that a failure is throwable and the + // caller can react. Inner scopes only record their success; the outermost one does the work. + if (m_database.m_transaction_depth == 1) { + auto database = QSqlDatabase::database(m_database.m_connection_name, false); + if (!database.commit()) { + throw DatabaseError("Cannot commit the transaction: {}", error_text(database.lastError())); + } + } + m_committed = true; +} + +Database::Transaction::~Transaction() +{ + if (!m_committed) { + m_database.m_transaction_poisoned = true; + } + --m_database.m_transaction_depth; + if (m_database.m_transaction_depth > 0) { + return; + } + + if (m_database.m_transaction_poisoned) { + // A destructor is implicitly noexcept, and both the rollback reporting and spdlog allocate, + // so anything thrown here would terminate the process. + try { + auto database = QSqlDatabase::database(m_database.m_connection_name, false); + if (!database.rollback()) { + spdlog::error("Failed to roll back the transaction: {}", error_text(database.lastError())); + } + } catch (...) { // NOLINT(bugprone-empty-catch) + } + } + m_database.m_transaction_poisoned = false; +} diff --git a/src/db/database.h b/src/db/database.h new file mode 100644 index 0000000..b67c9c2 --- /dev/null +++ b/src/db/database.h @@ -0,0 +1,109 @@ +#pragma once + +#include +#include + +class QSqlQuery; +class QThread; + +/** + * @class Database database.h "db/database.h" + * @brief Owns one SQL connection and the schema migrations applied to it. + * + * The class deliberately stores only the connection *name*, never a QSqlDatabase member. + * QSqlDatabase is a reference-counted handle, and QSqlDatabase::removeDatabase() warns (and + * leaks the connection) if any copy of the handle is still alive when it runs. Keeping only the + * name means every operation takes a short-lived local handle and the destructor can clean up. + * + * The connection name is unique per instance, so several independent in-memory databases can + * coexist in one process -- which is what lets each test have its own. + */ +class Database +{ +public: + enum class Dialect { SQLITE, POSTGRES }; + + /** + * @brief Opens (creating if absent) the database file at @p path. + * @throws DatabaseError if the driver is unavailable or the file cannot be opened. + */ + [[nodiscard]] static Database open_file(const std::filesystem::path& path); + + /** + * @brief Opens a private, empty, in-memory database. + * Each call yields a *distinct* database -- an in-memory connection is never shared. + */ + [[nodiscard]] static Database open_in_memory(); + + ~Database(); + Database(const Database&) = delete; + Database& operator=(const Database&) = delete; + Database(Database&& other) noexcept; + Database& operator=(Database&& other) noexcept; + + [[nodiscard]] Dialect dialect() const noexcept; + + /** + * @brief Applies every migration newer than the current schema version. + * @throws DatabaseError if the stored version is newer than this build understands. + */ + void migrate(); + + /** @brief Applies migrations up to (and including) @p target_version. Exposed for tests. */ + void migrate_to(int target_version); + + /** @brief The currently stored schema version, or 0 if the database is empty. */ + [[nodiscard]] int schema_version() const; + + /** @brief Whether a transaction scope is currently open. */ + [[nodiscard]] bool in_transaction() const noexcept; + + /** @throws DatabaseError if @p statement cannot be prepared. */ + [[nodiscard]] QSqlQuery prepare(const QString& statement) const; + + /** @throws DatabaseError if the query fails. */ + void execute(QSqlQuery& query) const; + + /** @brief Prepares and runs a statement taking no parameters. */ + void execute(const QString& statement) const; + + /** + * @class Transaction database.h "db/database.h" + * @brief RAII transaction scope, safe to nest. + * + * Only the outermost scope issues BEGIN and COMMIT; inner scopes just adjust a depth counter. + * That lets a model mutator open a transaction unconditionally without knowing whether an undo + * macro already opened one around it. + * + * If any scope is destroyed without commit() the whole nest is poisoned and the outermost one + * rolls back, so a failure deep inside a compound command cannot leave half of it committed. + */ + class Transaction + { + public: + explicit Transaction(Database& database); + ~Transaction(); + Transaction(const Transaction&) = delete; + Transaction& operator=(const Transaction&) = delete; + Transaction(Transaction&&) = delete; + Transaction& operator=(Transaction&&) = delete; + + void commit(); + + private: + Database& m_database; + bool m_committed = false; + }; + +private: + explicit Database(QString connection_name, Dialect dialect); + + /** @brief Throws DatabaseError if this is used from a thread other than the opening one. */ + void verify_thread() const; + + QString m_connection_name; + Dialect m_dialect = Dialect::SQLITE; + const QThread* m_thread = nullptr; + int m_transaction_depth = 0; + bool m_transaction_poisoned = false; +}; diff --git a/src/db/entityid.h b/src/db/entityid.h new file mode 100644 index 0000000..4ac834e --- /dev/null +++ b/src/db/entityid.h @@ -0,0 +1,46 @@ +#pragma once + +#include +#include + +/** + * @class EntityId entityid.h "db/entityid.h" + * @brief Opaque identity of a persisted entity. + * + * The id is a plain value and deliberately carries no dependency on Qt or SQL, so the domain + * types that hold one stay usable (and unit-testable) without a database. + * + * Ids are assigned by the application rather than by the database: SQLite's `INTEGER PRIMARY KEY` + * auto-assignment is rowid magic that PostgreSQL does not share, and `QSqlQuery::lastInsertId()` + * is driver-dependent. Assigning up-front also means the id is known before the INSERT, which is + * what lets an entity keep its original id when an undone removal is re-added. + * + * A default-constructed EntityId is invalid and denotes an entity that has never been persisted. + */ +class EntityId +{ +public: + using Value = std::int64_t; + + // Not explicit: Plan::Entry is an aggregate, and initializing it without naming the id + // value-initializes this member, which an explicit default constructor would forbid. + EntityId() noexcept = default; + explicit EntityId(Value value) noexcept : m_value(value) + { + } + + [[nodiscard]] Value value() const noexcept + { + return m_value; + } + + [[nodiscard]] bool is_valid() const noexcept + { + return m_value != 0; + } + + friend auto operator<=>(const EntityId&, const EntityId&) noexcept = default; + +private: + Value m_value = 0; +}; diff --git a/src/db/enumnames.cpp b/src/db/enumnames.cpp new file mode 100644 index 0000000..286aeea --- /dev/null +++ b/src/db/enumnames.cpp @@ -0,0 +1,88 @@ +#include "db/enumnames.h" + +#include "exceptions.h" + +#include +#include +#include +#include + +namespace +{ + +// Keeping the tables next to the switches means from_db_name and db_name cannot drift apart. +constexpr auto period_type_names = std::array{ + std::pair{Period::Type::Year, "YEAR"}, std::pair{Period::Type::Month, "MONTH"}, + std::pair{Period::Type::Week, "WEEK"}, std::pair{Period::Type::Day, "DAY"}, + std::pair{Period::Type::Custom, "CUSTOM"}, +}; + +constexpr auto plan_kind_names = std::array{ + std::pair{Plan::Kind::Normal, "NORMAL"}, + std::pair{Plan::Kind::Sick, "SICK"}, + std::pair{Plan::Kind::Holiday, "HOLIDAY"}, + std::pair{Plan::Kind::HalfHoliday, "HALF_HOLIDAY"}, + std::pair{Plan::Kind::Vacation, "VACATION"}, + std::pair{Plan::Kind::HalfVacation, "HALF_VACATION"}, + std::pair{Plan::Kind::HalfVacationHalfHoliday, "HALF_VACATION_HALF_HOLIDAY"}, +}; + +template [[nodiscard]] Enum +from_name(const std::array, n>& names, const QString& name, const char* const type_name) +{ + const auto it = std::ranges::find(names, name, [](const auto& pair) { return QString::fromLatin1(pair.second); }); + if (it == names.end()) { + throw DatabaseError("'{}' is not a valid {}.", name.toStdString(), type_name); + } + return it->first; +} + +} // namespace + +QString db_name(const Period::Type type) +{ + switch (type) { + case Period::Type::Year: + return QStringLiteral("YEAR"); + case Period::Type::Month: + return QStringLiteral("MONTH"); + case Period::Type::Week: + return QStringLiteral("WEEK"); + case Period::Type::Day: + return QStringLiteral("DAY"); + case Period::Type::Custom: + return QStringLiteral("CUSTOM"); + } + throw DatabaseError("Unknown Period::Type: {}.", static_cast(type)); +} + +QString db_name(const Plan::Kind kind) +{ + switch (kind) { + case Plan::Kind::Normal: + return QStringLiteral("NORMAL"); + case Plan::Kind::Sick: + return QStringLiteral("SICK"); + case Plan::Kind::Holiday: + return QStringLiteral("HOLIDAY"); + case Plan::Kind::HalfHoliday: + return QStringLiteral("HALF_HOLIDAY"); + case Plan::Kind::Vacation: + return QStringLiteral("VACATION"); + case Plan::Kind::HalfVacation: + return QStringLiteral("HALF_VACATION"); + case Plan::Kind::HalfVacationHalfHoliday: + return QStringLiteral("HALF_VACATION_HALF_HOLIDAY"); + } + throw DatabaseError("Unknown Plan::Kind: {}.", static_cast(kind)); +} + +Period::Type period_type_from_db_name(const QString& name) +{ + return ::from_name(period_type_names, name, "Period::Type"); +} + +Plan::Kind plan_kind_from_db_name(const QString& name) +{ + return ::from_name(plan_kind_names, name, "Plan::Kind"); +} diff --git a/src/db/enumnames.h b/src/db/enumnames.h new file mode 100644 index 0000000..3bf562f --- /dev/null +++ b/src/db/enumnames.h @@ -0,0 +1,23 @@ +#pragma once + +#include "period.h" +#include "plan.h" + +class QString; + +/** + * @brief Stable, human-readable names under which enumerators are stored in the database. + * + * These are deliberately *not* the fmt formatters used for display: those go through + * QObject::tr(), so they change with the user's language, and Period::Type::Custom has no display + * label at all. Persistence names must never move. + * + * The implementations switch exhaustively with no default label, so adding an enumerator becomes + * a compile error here rather than a silent data-loss bug. + */ +[[nodiscard]] QString db_name(Period::Type type); +[[nodiscard]] QString db_name(Plan::Kind kind); + +/** @throws DatabaseError if @p name is not a known enumerator name. */ +[[nodiscard]] Period::Type period_type_from_db_name(const QString& name); +[[nodiscard]] Plan::Kind plan_kind_from_db_name(const QString& name); diff --git a/src/db/migrations.cpp b/src/db/migrations.cpp new file mode 100644 index 0000000..ecdcb47 --- /dev/null +++ b/src/db/migrations.cpp @@ -0,0 +1,96 @@ +#include "db/migrations.h" + +#include "db/database.h" + +#include +#include + +namespace +{ + +void apply_initial_schema(Database& database) +{ + // The DDL below sticks to constructs SQLite and PostgreSQL share, so that switching the Qt + // driver to QPSQL later stays a connection-string change: + // - no INTEGER PRIMARY KEY autoincrement (SQLite rowid magic) and no SERIAL (PostgreSQL only); + // ids are assigned by the application, see EntityId. + // - TEXT for dates, times and colors; BIGINT for counts. + // - named constraints, so a later migration can refer to them. + database.execute(QStringLiteral(R"( + CREATE TABLE schema_version ( + version INTEGER NOT NULL, + applied_at TEXT NOT NULL, + CONSTRAINT pk_schema_version PRIMARY KEY (version) + ))")); + + database.execute(QStringLiteral(R"( + CREATE TABLE project ( + id BIGINT NOT NULL, + name TEXT NOT NULL, + color TEXT NOT NULL, + CONSTRAINT pk_project PRIMARY KEY (id) + ))")); + // No UNIQUE on name yet: ProjectModel::add currently accepts duplicates, and a constraint the + // in-memory model does not enforce would reject a state the model happily produces -- failing + // only after the mutation already happened. Add it once the model enforces uniqueness. + + database.execute(QStringLiteral(R"( + CREATE TABLE interval ( + id BIGINT NOT NULL, + project_id BIGINT NULL, + begin_time TEXT NULL, + end_time TEXT NULL, + CONSTRAINT pk_interval PRIMARY KEY (id), + CONSTRAINT fk_interval_project FOREIGN KEY (project_id) + REFERENCES project (id) ON DELETE RESTRICT + ))")); + // end_time IS NULL means the interval is still running. + // + // Deliberately no CHECK (end_time >= begin_time). Retiming an interval is two commands -- + // swap_begin then swap_end -- and each writes through, so the row legitimately passes through a + // state whose begin is after its end. A constraint the in-memory model does not enforce would + // reject a state the model routinely produces, and the write would fail after the change had + // already happened in memory. Same reasoning as the missing UNIQUE on project.name above. + database.execute(QStringLiteral("CREATE INDEX ix_interval_begin_time ON interval (begin_time)")); + database.execute(QStringLiteral("CREATE INDEX ix_interval_project_id ON interval (project_id)")); + + database.execute(QStringLiteral(R"( + CREATE TABLE plan_entry ( + id BIGINT NOT NULL, + period_begin TEXT NOT NULL, + period_end TEXT NOT NULL, + period_type TEXT NOT NULL, + kind TEXT NOT NULL, + CONSTRAINT pk_plan_entry PRIMARY KEY (id), + CONSTRAINT ck_plan_entry_order CHECK (period_end >= period_begin), + CONSTRAINT ck_plan_entry_type CHECK (period_type IN ('YEAR','MONTH','WEEK','DAY','CUSTOM')), + CONSTRAINT ck_plan_entry_kind CHECK (kind IN ('NORMAL','SICK','HOLIDAY','HALF_HOLIDAY', + 'VACATION','HALF_VACATION','HALF_VACATION_HALF_HOLIDAY')) + ))")); + database.execute(QStringLiteral("CREATE INDEX ix_plan_entry_period_begin ON plan_entry (period_begin)")); + + database.execute(QStringLiteral(R"( + CREATE TABLE plan_setting ( + id BIGINT NOT NULL, + start_date TEXT NOT NULL, + overtime_offset_minutes BIGINT NOT NULL, + CONSTRAINT pk_plan_setting PRIMARY KEY (id), + CONSTRAINT ck_plan_setting_singleton CHECK (id = 1) + ))")); +} + +constexpr auto all_migrations = std::array{ + Migration{.version = 1, .description = "initial schema", .apply = &apply_initial_schema}, +}; + +} // namespace + +std::span migrations() +{ + return all_migrations; +} + +int latest_schema_version() +{ + return all_migrations.back().version; +} diff --git a/src/db/migrations.h b/src/db/migrations.h new file mode 100644 index 0000000..01963b9 --- /dev/null +++ b/src/db/migrations.h @@ -0,0 +1,26 @@ +#pragma once + +#include +#include + +class Database; + +/** + * @struct Migration migrations.h "db/migrations.h" + * @brief One forward step of the database schema. + * + * Migrations are applied in ascending version order and each one runs in its own transaction, so + * an interrupted upgrade leaves the database at the last version that fully succeeded. + */ +struct Migration +{ + int version; + std::string_view description; + void (*apply)(Database& database); +}; + +/** @brief All known migrations, sorted ascending by version. */ +[[nodiscard]] std::span migrations(); + +/** @brief The version a fully migrated database ends up at. */ +[[nodiscard]] int latest_schema_version(); diff --git a/src/db/sqltimesheetrepository.cpp b/src/db/sqltimesheetrepository.cpp new file mode 100644 index 0000000..5ccccb3 --- /dev/null +++ b/src/db/sqltimesheetrepository.cpp @@ -0,0 +1,278 @@ +#include "db/sqltimesheetrepository.h" + +#include "application.h" +#include "db/database.h" +#include "db/enumnames.h" +#include "db/sqlvalue.h" +#include "exceptions.h" +#include "interval.h" +#include "intervalmodel.h" +#include "plan.h" +#include "project.h" +#include "projectmodel.h" +#include "timesheet.h" + +#include +#include +#include +#include + +namespace +{ + +constexpr auto plan_setting_id = 1; + +[[nodiscard]] QVariant id_to_sql(const EntityId id) +{ + return QVariant::fromValue(static_cast(id.value())); +} + +/** @brief The id of the project an interval points at, or null when it has none. */ +[[nodiscard]] QVariant project_reference(const Interval& interval) +{ + if (interval.project() == nullptr) { + return {}; + } + return ::id_to_sql(interval.project()->id()); +} + +} // namespace + +SqlTimeSheetRepository::SqlTimeSheetRepository(Database& database) : m_database(database) +{ + seed_id_counters(); +} + +void SqlTimeSheetRepository::seed_id_counters() +{ + // Seeding here rather than in load() keeps a repository that only ever inserts (as in tests) + // just as safe as one that loaded first. + const auto highest_id = [this](const QString& table) { + auto query = m_database.prepare(QStringLiteral("SELECT COALESCE(MAX(id), 0) FROM %1").arg(table)); + m_database.execute(query); + if (!query.next()) { + return EntityId::Value{0}; + } + return static_cast(query.value(0).toLongLong()); + }; + m_next_project_id = highest_id(QStringLiteral("project")); + m_next_interval_id = highest_id(QStringLiteral("interval")); + m_next_plan_entry_id = highest_id(QStringLiteral("plan_entry")); +} + +EntityId SqlTimeSheetRepository::next_id(EntityId::Value& counter) const noexcept +{ + ++counter; + return EntityId{counter}; +} + +void SqlTimeSheetRepository::insert(Project& project) +{ + if (!project.id().is_valid()) { + project.set_id(next_id(m_next_project_id)); + } + // Upsert rather than a plain INSERT: after a rolled-back delete the row still exists while the + // entity is gone from memory, and an undo would otherwise collide on the primary key. + auto query = + m_database.prepare(QStringLiteral("INSERT INTO project (id, name, color) VALUES (?, ?, ?) " + "ON CONFLICT (id) DO UPDATE SET name = excluded.name, color = excluded.color")); + query.addBindValue(::id_to_sql(project.id())); + query.addBindValue(project.name()); + query.addBindValue(to_sql(project.color())); + m_database.execute(query); +} + +void SqlTimeSheetRepository::update(const Project& project) +{ + auto query = m_database.prepare(QStringLiteral("UPDATE project SET name = ?, color = ? WHERE id = ?")); + query.addBindValue(project.name()); + query.addBindValue(to_sql(project.color())); + query.addBindValue(::id_to_sql(project.id())); + m_database.execute(query); +} + +void SqlTimeSheetRepository::remove(const Project& project) +{ + auto query = m_database.prepare(QStringLiteral("DELETE FROM project WHERE id = ?")); + query.addBindValue(::id_to_sql(project.id())); + m_database.execute(query); +} + +void SqlTimeSheetRepository::insert(Interval& interval) +{ + if (!interval.id().is_valid()) { + interval.set_id(next_id(m_next_interval_id)); + } + auto query = m_database.prepare( + QStringLiteral("INSERT INTO interval (id, project_id, begin_time, end_time) VALUES (?, ?, ?, ?) " + "ON CONFLICT (id) DO UPDATE SET project_id = excluded.project_id, " + "begin_time = excluded.begin_time, end_time = excluded.end_time")); + query.addBindValue(::id_to_sql(interval.id())); + query.addBindValue(::project_reference(interval)); + query.addBindValue(to_sql(interval.begin())); + query.addBindValue(to_sql(interval.end())); + m_database.execute(query); +} + +void SqlTimeSheetRepository::update(const Interval& interval) +{ + auto query = m_database.prepare( + QStringLiteral("UPDATE interval SET project_id = ?, begin_time = ?, end_time = ? WHERE id = ?")); + query.addBindValue(::project_reference(interval)); + query.addBindValue(to_sql(interval.begin())); + query.addBindValue(to_sql(interval.end())); + query.addBindValue(::id_to_sql(interval.id())); + m_database.execute(query); +} + +void SqlTimeSheetRepository::remove(const Interval& interval) +{ + auto query = m_database.prepare(QStringLiteral("DELETE FROM interval WHERE id = ?")); + query.addBindValue(::id_to_sql(interval.id())); + m_database.execute(query); +} + +void SqlTimeSheetRepository::insert(Plan::Entry& entry) +{ + if (!entry.id.is_valid()) { + entry.id = next_id(m_next_plan_entry_id); + } + auto query = + m_database.prepare(QStringLiteral("INSERT INTO plan_entry (id, period_begin, period_end, period_type, kind) " + "VALUES (?, ?, ?, ?, ?) ON CONFLICT (id) DO UPDATE SET " + "period_begin = excluded.period_begin, period_end = excluded.period_end, " + "period_type = excluded.period_type, kind = excluded.kind")); + query.addBindValue(::id_to_sql(entry.id)); + query.addBindValue(to_sql(entry.period.begin())); + query.addBindValue(to_sql(entry.period.end())); + query.addBindValue(db_name(entry.period.type())); + query.addBindValue(db_name(entry.kind)); + m_database.execute(query); +} + +void SqlTimeSheetRepository::update(const Plan::Entry& entry) +{ + auto query = m_database.prepare(QStringLiteral("UPDATE plan_entry SET period_begin = ?, period_end = ?, " + "period_type = ?, kind = ? WHERE id = ?")); + query.addBindValue(to_sql(entry.period.begin())); + query.addBindValue(to_sql(entry.period.end())); + query.addBindValue(db_name(entry.period.type())); + query.addBindValue(db_name(entry.kind)); + query.addBindValue(::id_to_sql(entry.id)); + m_database.execute(query); +} + +void SqlTimeSheetRepository::remove(const Plan::Entry& entry) +{ + auto query = m_database.prepare(QStringLiteral("DELETE FROM plan_entry WHERE id = ?")); + query.addBindValue(::id_to_sql(entry.id)); + m_database.execute(query); +} + +void SqlTimeSheetRepository::update_plan_setting(const Plan& plan) +{ + // Upsert, so a repository used without load() (which is what creates the row) still stores the + // settings instead of silently updating nothing. + auto query = m_database.prepare( + QStringLiteral("INSERT INTO plan_setting (id, start_date, overtime_offset_minutes) VALUES (?, ?, ?) " + "ON CONFLICT (id) DO UPDATE SET start_date = excluded.start_date, " + "overtime_offset_minutes = excluded.overtime_offset_minutes")); + query.addBindValue(plan_setting_id); + query.addBindValue(to_sql(plan.start())); + query.addBindValue(to_sql(plan.overtime_offset())); + m_database.execute(query); +} + +std::unique_ptr SqlTimeSheetRepository::load() +{ + // One transaction so the four reads see a consistent snapshot of the database. + Database::Transaction transaction{m_database}; + + auto projects = std::vector>{}; + auto projects_by_id = std::unordered_map{}; + { + auto query = m_database.prepare(QStringLiteral("SELECT id, name, color FROM project ORDER BY id")); + m_database.execute(query); + while (query.next()) { + auto project = std::make_unique(query.value(1).toString(), color_from_sql(query.value(2))); + project->set_id(EntityId{query.value(0).toLongLong()}); + projects_by_id.emplace(project->id().value(), project.get()); + projects.push_back(std::move(project)); + } + } + + auto intervals = std::deque>{}; + { + auto query = + m_database.prepare(QStringLiteral("SELECT id, project_id, begin_time, end_time FROM interval ORDER BY id")); + m_database.execute(query); + while (query.next()) { + // Resolving by id replaces the old JSON format's positional reference, which silently + // mis-associated intervals whenever the project order changed. + const auto* project = static_cast(nullptr); + if (!query.value(1).isNull()) { + const auto project_id = static_cast(query.value(1).toLongLong()); + const auto it = projects_by_id.find(project_id); + if (it == projects_by_id.end()) { + throw DatabaseError("Interval {} references project {}, which does not exist.", query.value(0).toLongLong(), + project_id); + } + project = it->second; + } + auto interval = std::make_unique(project); + interval->swap_begin(date_time_from_sql(query.value(2))); + interval->swap_end(date_time_from_sql(query.value(3))); + interval->set_id(EntityId{query.value(0).toLongLong()}); + intervals.push_back(std::move(interval)); + } + } + + auto entries = std::vector>{}; + { + auto query = m_database.prepare(QStringLiteral( + "SELECT id, period_begin, period_end, period_type, kind FROM plan_entry ORDER BY period_begin, id")); + m_database.execute(query); + while (query.next()) { + const auto begin = date_from_sql(query.value(1)); + const auto end = date_from_sql(query.value(2)); + const auto type = period_type_from_db_name(query.value(3).toString()); + // A typed period recomputes its end from its begin; only a custom one carries both. + auto period = (type == Period::Type::Custom) ? Period{begin, end} : Period{begin, type}; + if (period.end() != end) { + spdlog::warn("Stored period end {} disagrees with the recomputed end {}; using the recomputed one.", + end.toString(Qt::ISODate).toStdString(), period.end().toString(Qt::ISODate).toStdString()); + } + entries.push_back(std::make_unique(period, plan_kind_from_db_name(query.value(4).toString()), + EntityId{query.value(0).toLongLong()})); + } + } + + auto start = Application::current_date_time().date(); + auto overtime_offset = std::chrono::minutes{0}; + { + auto query = + m_database.prepare(QStringLiteral("SELECT start_date, overtime_offset_minutes FROM plan_setting WHERE id = ?")); + query.addBindValue(plan_setting_id); + m_database.execute(query); + if (query.next()) { + start = date_from_sql(query.value(0)); + overtime_offset = minutes_from_sql(query.value(1)); + } else { + // Fresh database. Pinning the start date now matters: without a stored row, every launch + // would silently reset it to "today" and the overtime balance would never accumulate. + auto insert = m_database.prepare( + QStringLiteral("INSERT INTO plan_setting (id, start_date, overtime_offset_minutes) VALUES (?, ?, ?)")); + insert.addBindValue(plan_setting_id); + insert.addBindValue(to_sql(start)); + insert.addBindValue(to_sql(overtime_offset)); + m_database.execute(insert); + } + } + + auto time_sheet = + std::make_unique(std::make_unique(*this, std::move(projects)), + std::make_unique(*this, std::move(intervals)), + std::make_unique(*this, start, overtime_offset, std::move(entries))); + transaction.commit(); + return time_sheet; +} diff --git a/src/db/sqltimesheetrepository.h b/src/db/sqltimesheetrepository.h new file mode 100644 index 0000000..943cd65 --- /dev/null +++ b/src/db/sqltimesheetrepository.h @@ -0,0 +1,54 @@ +#pragma once + +#include "db/abstracttimesheetrepository.h" +#include "db/entityid.h" + +#include + +class Database; +class TimeSheet; + +/** + * @class SqlTimeSheetRepository sqltimesheetrepository.h "db/sqltimesheetrepository.h" + * @brief Reads and writes the timesheet through a SQL Database. + * + * Ids are handed out by this class rather than by the database, so that they are known before + * the INSERT and stay portable across SQL dialects. The counters are seeded from the highest id + * already stored, which happens in the constructor so that a repository used without load() is + * equally safe. + */ +class SqlTimeSheetRepository final : public AbstractTimeSheetRepository +{ +public: + explicit SqlTimeSheetRepository(Database& database); + + /** + * @brief Reads the whole timesheet into memory. + * Creates the plan settings row if this is a fresh database. + * @throws DatabaseError if the stored data cannot be interpreted. + */ + [[nodiscard]] std::unique_ptr load(); + + void insert(Project& project) override; + void update(const Project& project) override; + void remove(const Project& project) override; + + void insert(Interval& interval) override; + void update(const Interval& interval) override; + void remove(const Interval& interval) override; + + void insert(Plan::Entry& entry) override; + void update(const Plan::Entry& entry) override; + void remove(const Plan::Entry& entry) override; + + void update_plan_setting(const Plan& plan) override; + +private: + [[nodiscard]] EntityId next_id(EntityId::Value& counter) const noexcept; + void seed_id_counters(); + + Database& m_database; + EntityId::Value m_next_project_id = 0; + EntityId::Value m_next_interval_id = 0; + EntityId::Value m_next_plan_entry_id = 0; +}; diff --git a/src/db/sqlvalue.cpp b/src/db/sqlvalue.cpp new file mode 100644 index 0000000..c964f47 --- /dev/null +++ b/src/db/sqlvalue.cpp @@ -0,0 +1,95 @@ +#include "db/sqlvalue.h" + +#include "exceptions.h" + +#include +#include +#include + +namespace +{ + +// Fixed-width, second-precision, no timezone suffix. The domain works exclusively in local time +// (QDateTime::currentDateTime(), QDate::startOfDay()), so storing bare local time avoids +// conversion surprises, and the fixed width keeps string comparison equivalent to date +// comparison. +constexpr auto date_time_format = "yyyy-MM-ddTHH:mm:ss"; +constexpr auto date_format = "yyyy-MM-dd"; + +} // namespace + +QVariant to_sql(const QDateTime& date_time) +{ + if (!date_time.isValid()) { + return {}; + } + return date_time.toString(date_time_format); +} + +QVariant to_sql(const QDate& date) +{ + if (!date.isValid()) { + return {}; + } + return date.toString(date_format); +} + +QVariant to_sql(const QColor& color) +{ + if (!color.isValid()) { + return {}; + } + return color.name(QColor::HexArgb); +} + +QVariant to_sql(const std::chrono::minutes minutes) +{ + return QVariant::fromValue(static_cast(minutes.count())); +} + +QDateTime date_time_from_sql(const QVariant& value) +{ + if (value.isNull()) { + return {}; + } + auto date_time = QDateTime::fromString(value.toString(), date_time_format); + if (!date_time.isValid()) { + throw DatabaseError("Cannot read '{}' as a date and time.", value.toString().toStdString()); + } + return date_time; +} + +QDate date_from_sql(const QVariant& value) +{ + if (value.isNull()) { + return {}; + } + auto date = QDate::fromString(value.toString(), date_format); + if (!date.isValid()) { + throw DatabaseError("Cannot read '{}' as a date.", value.toString().toStdString()); + } + return date; +} + +QColor color_from_sql(const QVariant& value) +{ + if (value.isNull()) { + return {}; + } + // QColor::fromString accepts both '#rrggbb' and '#aarrggbb'. + auto color = QColor::fromString(value.toString()); + if (!color.isValid()) { + throw DatabaseError("Cannot read '{}' as a color.", value.toString().toStdString()); + } + return color; +} + +std::chrono::minutes minutes_from_sql(const QVariant& value) +{ + auto ok = false; + const auto count = value.toLongLong(&ok); + if (!ok) { + throw DatabaseError("Cannot read '{}' as a number of minutes.", value.toString().toStdString()); + } + return std::chrono::minutes{count}; +} diff --git a/src/db/sqlvalue.h b/src/db/sqlvalue.h new file mode 100644 index 0000000..10cbff3 --- /dev/null +++ b/src/db/sqlvalue.h @@ -0,0 +1,34 @@ +#pragma once + +#include + +class QColor; +class QDate; +class QDateTime; +class QString; +class QVariant; + +/** + * @brief Conversions between domain values and the QVariants bound to / read from SQL. + * + * Everything is stored in types both SQLite and PostgreSQL share, so the schema survives a + * driver swap: + * - QDateTime / QDate become fixed-width ISO-8601 TEXT. Fixed width matters: it makes a + * lexicographic `BETWEEN` on begin_time a correct chronological range scan. + * - QColor becomes '#aarrggbb' TEXT. Note this keeps the alpha channel, which the previous + * JSON format silently dropped by using QColor::name(). + * - std::chrono::minutes becomes a plain integer count, which may be negative + * (Plan::overtime_offset can be). + * + * An invalid QDateTime / QDate maps to a null QVariant and back. That round-trip is what + * expresses "this interval is still running" (interval.end_time IS NULL). + */ +[[nodiscard]] QVariant to_sql(const QDateTime& date_time); +[[nodiscard]] QVariant to_sql(const QDate& date); +[[nodiscard]] QVariant to_sql(const QColor& color); +[[nodiscard]] QVariant to_sql(std::chrono::minutes minutes); + +[[nodiscard]] QDateTime date_time_from_sql(const QVariant& value); +[[nodiscard]] QDate date_from_sql(const QVariant& value); +[[nodiscard]] QColor color_from_sql(const QVariant& value); +[[nodiscard]] std::chrono::minutes minutes_from_sql(const QVariant& value); diff --git a/src/enum.h b/src/enum.h deleted file mode 100644 index 25d4817..0000000 --- a/src/enum.h +++ /dev/null @@ -1,15 +0,0 @@ -#pragma once - -#include "exceptions.h" -#include "fmt.h" - -template size> EnumT enum_from_string(const std::string& s) -{ - for (std::underlying_type_t i = 0; i < size; ++i) { - const auto e_val = static_cast(i); - if (fmt::format("{}", e_val) == s) { - return e_val; - } - } - throw RuntimeError("Failed to parse Enum ({})", s); -} diff --git a/src/exceptions.h b/src/exceptions.h index 0cc2c8f..8a4512e 100644 --- a/src/exceptions.h +++ b/src/exceptions.h @@ -12,13 +12,12 @@ class RuntimeError : public std::runtime_error } }; -class DeserializationError final : public RuntimeError -{ -public: - using RuntimeError::RuntimeError; -}; - -class InvalidEnumNameException final : public RuntimeError +/** + * @brief Thrown when the database cannot be opened, migrated, read or written. + * Derives from RuntimeError so call sites that already report a RuntimeError to the user + * (e.g. PlanTableView::open_period_edit) handle persistence failures without change. + */ +class DatabaseError final : public RuntimeError { public: using RuntimeError::RuntimeError; diff --git a/src/interval.cpp b/src/interval.cpp index 9aeee54..ab46cf8 100644 --- a/src/interval.cpp +++ b/src/interval.cpp @@ -1,11 +1,8 @@ #include "interval.h" #include "application.h" #include "exceptions.h" -#include "json.h" #include "period.h" -#include - Interval::Interval(const Project* project) : m_project(project) { } @@ -74,6 +71,16 @@ Period Interval::period() const return Period{m_begin.date(), m_end.date()}; } +EntityId Interval::id() const noexcept +{ + return m_id; +} + +void Interval::set_id(const EntityId id) noexcept +{ + m_id = id; +} + std::weak_ordering operator<=>(const Interval& a, const Interval& b) noexcept { if (a.begin() == b.begin()) { diff --git a/src/interval.h b/src/interval.h index d97c820..17822b1 100644 --- a/src/interval.h +++ b/src/interval.h @@ -1,8 +1,9 @@ #pragma once +#include "db/entityid.h" + #include #include -#include class Project; class Period; @@ -11,7 +12,8 @@ class Period; * @class Interval interval.h "interval.h" * @brief A Period represents a named time range with well-defined begin and optional end. * At first glance, it resembles the Period, however, its purpose is very different. - * - In contrast to a Period, an Interval can be serialized. + * - In contrast to a Period, an Interval is a stored entity with its own identity (see EntityId); + * a Period is a plain value. * - The Interval usually reflects a particular time range defined only within * this timesheet (e.g., Friday, November 1st from 8:15 to 10:23). * - The Interval has a precision of a minute, while Period is precise only up to a day. @@ -25,18 +27,37 @@ class Interval friend std::weak_ordering operator<=>(const Interval& a, const Interval& b) noexcept; explicit Interval(const Project* project); + + /** + * @name Mutators + * Each swaps in a new value and returns the previous one, which is what makes them usable as + * their own inverse in a ModifyCommand. + * + * @warning Mutating an Interval that an IntervalModel already owns must go through + * make_modify_interval_command(), or the change will not reach the database. These setters are + * deliberately unaware of persistence, because an Interval is routinely built and filled in + * before it is added to a model, when there is no row to update yet. + * @{ + */ const Project* swap_project(const Project* project) noexcept; - [[nodiscard]] const Project* project() const noexcept; QDateTime swap_begin(QDateTime begin); QDateTime swap_end(QDateTime end); + /** @} */ + + [[nodiscard]] const Project* project() const noexcept; [[nodiscard]] const QDateTime& begin() const noexcept; [[nodiscard]] const QDateTime& end() const noexcept; [[nodiscard]] QString duration_text() const; [[nodiscard]] std::chrono::minutes duration() const; [[nodiscard]] Period period() const; + /** @brief Identity of this interval's row, invalid until it has been persisted. */ + [[nodiscard]] EntityId id() const noexcept; + void set_id(EntityId id) noexcept; + private: const Project* m_project; QDateTime m_begin; QDateTime m_end; + EntityId m_id; }; diff --git a/src/intervalmodel.cpp b/src/intervalmodel.cpp index 0e98f64..90d9686 100644 --- a/src/intervalmodel.cpp +++ b/src/intervalmodel.cpp @@ -1,9 +1,10 @@ #include "intervalmodel.h" #include "colorutil.h" +#include "db/abstracttimesheetrepository.h" +#include "exceptions.h" #include "period.h" #include #include -#include #include namespace @@ -36,7 +37,16 @@ template [[nodiscard]] auto find(Intervals&& intervals, cons } // namespace -IntervalModel::IntervalModel(std::deque> intervals) : m_intervals(std::move(intervals)) +IntervalModel::IntervalModel() : IntervalModel(null_repository()) +{ +} + +IntervalModel::IntervalModel(AbstractTimeSheetRepository& repository) : m_repository(repository) +{ +} + +IntervalModel::IntervalModel(AbstractTimeSheetRepository& repository, std::deque> intervals) + : m_repository(repository), m_intervals(std::move(intervals)) { } @@ -131,27 +141,23 @@ void IntervalModel::add(std::unique_ptr interval) { const auto row = static_cast(m_intervals.size()); beginInsertRows({}, row, row); - m_intervals.emplace_back(std::move(interval)); + auto& ref = *m_intervals.emplace_back(std::move(interval)); endInsertRows(); + // An interval that already carries an id is one an undone removal is putting back; insert() + // reuses it so the row returns to where it was. + m_repository.insert(ref); Q_EMIT data_changed(); } -void IntervalModel::split_interval(const Interval& interval, const QDateTime& split_point) -{ - const auto location = ::find(m_intervals, interval); - auto& left_interval = **location.iterator; - beginInsertRows({}, location.row, location.row); - const auto& right_interval = - *m_intervals.emplace(std::next(location.iterator), std::make_unique(interval.project())); - endInsertRows(); - right_interval->swap_end(left_interval.end()); - left_interval.swap_end(split_point); - right_interval->swap_begin(split_point); -} - std::unique_ptr IntervalModel::extract(const Interval& interval) { const auto location = ::find(m_intervals, interval); + if (location.iterator == m_intervals.end()) { + throw RuntimeError("Cannot extract an interval that this model does not own."); + } + // Delete the row before touching the container, so a failure leaves the model untouched rather + // than destroying the interval during unwinding and dangling the calling command's reference. + m_repository.remove(interval); beginRemoveRows({}, location.row, location.row); auto extracted_interval = std::move(*location.iterator); m_intervals.erase(location.iterator); @@ -160,6 +166,11 @@ std::unique_ptr IntervalModel::extract(const Interval& interval) return extracted_interval; } +void IntervalModel::persist(const Interval& interval) +{ + m_repository.update(interval); +} + void IntervalModel::set_intervals(std::deque> intervals) { beginResetModel(); diff --git a/src/intervalmodel.h b/src/intervalmodel.h index 2954473..ec0b1c8 100644 --- a/src/intervalmodel.h +++ b/src/intervalmodel.h @@ -6,12 +6,17 @@ #include #include +class AbstractTimeSheetRepository; + class IntervalModel final : public QAbstractTableModel { Q_OBJECT public: - explicit IntervalModel(std::deque> intervals); - explicit IntervalModel() = default; + /** @brief Creates a model that does not persist anything. */ + explicit IntervalModel(); + explicit IntervalModel(AbstractTimeSheetRepository& repository); + /** @brief Adopts already-stored intervals without writing them back. */ + explicit IntervalModel(AbstractTimeSheetRepository& repository, std::deque> intervals); static constexpr auto project_column = 0; static constexpr auto date_column = 1; @@ -34,7 +39,14 @@ class IntervalModel final : public QAbstractTableModel void add(std::unique_ptr interval); std::unique_ptr extract(const Interval& interval); - void split_interval(const Interval& interval, const QDateTime& split_point); + + /** + * @brief Writes @p interval's current state to the store. + * Interval's own setters cannot do this: an Interval is routinely filled in before it belongs + * to a model, when there is no row yet. Call this after mutating an owned interval -- + * make_modify_interval_command() does so for every undoable edit. + */ + void persist(const Interval& interval); void set_intervals(std::deque> intervals); [[nodiscard]] std::vector intervals() const; @@ -46,6 +58,7 @@ class IntervalModel final : public QAbstractTableModel void data_changed(); private: + AbstractTimeSheetRepository& m_repository; std::deque> m_intervals; [[nodiscard]] QVariant background_data(const QModelIndex& index) const; }; diff --git a/src/json.cpp b/src/json.cpp deleted file mode 100644 index a10cb76..0000000 --- a/src/json.cpp +++ /dev/null @@ -1,61 +0,0 @@ -#include "json.h" - -#include -#include - -namespace nlohmann -{ - -void adl_serializer::to_json(json& j, const QString& value) -{ - j = value.toStdString(); -} - -void adl_serializer::from_json(const json& j, QString& value) -{ - value = QString::fromStdString(static_cast(j)); -} - -void adl_serializer::to_json(json& j, const std::chrono::minutes& value) -{ - j = value.count(); -} - -void adl_serializer::from_json(const json& j, std::chrono::minutes& value) -{ - using std::chrono_literals::operator""min; - value = j.get() * 1min; -} - -void adl_serializer::to_json(json& j, const QDate& value) -{ - j = value.toString(Qt::ISODate); -} - -void adl_serializer::from_json(const json& j, QDate& value) -{ - value = QDate::fromString(j.get(), Qt::ISODate); -} - -void adl_serializer::to_json(json& j, const QDateTime& value) -{ - j = value.toString(Qt::ISODate); -} - -void adl_serializer::from_json(const json& j, QDateTime& value) -{ - value = QDateTime::fromString(j.get(), Qt::ISODate); -} - -void adl_serializer, void>::to_json(json& j, const QStringList& value) -{ - j = std::vector(value.begin(), value.end()); -} - -void adl_serializer, void>::from_json(const json& j, QStringList& value) -{ - auto vec = j.get>(); - value = QStringList(vec.begin(), vec.end()); -} - -} // namespace nlohmann diff --git a/src/json.h b/src/json.h deleted file mode 100644 index 571591c..0000000 --- a/src/json.h +++ /dev/null @@ -1,39 +0,0 @@ -#pragma once - -#include -#include -#include - -namespace nlohmann -{ -template<> struct adl_serializer -{ - static void to_json(json& j, const QString& value); - static void from_json(const json& j, QString& value); -}; - -template<> struct adl_serializer -{ - static void to_json(json& j, const std::chrono::minutes& value); - static void from_json(const json& j, std::chrono::minutes& value); -}; - -template<> struct adl_serializer -{ - static void to_json(json& j, const QDate& value); - static void from_json(const json& j, QDate& value); -}; - -template<> struct adl_serializer -{ - static void to_json(json& j, const QDateTime& value); - static void from_json(const json& j, QDateTime& value); -}; - -template<> struct adl_serializer -{ - static void to_json(json& j, const QStringList& value); - static void from_json(const json& j, QStringList& value); -}; - -} // namespace nlohmann diff --git a/src/kdsingleapplication/kdsingleapplication_lib.h b/src/kdsingleapplication/kdsingleapplication_lib.h new file mode 100644 index 0000000..950d326 --- /dev/null +++ b/src/kdsingleapplication/kdsingleapplication_lib.h @@ -0,0 +1,23 @@ +/* + This file is part of KDSingleApplication. + + SPDX-FileCopyrightText: 2019 Klarälvdalens Datakonsult AB, a KDAB Group company + + SPDX-License-Identifier: MIT + + Contact KDAB at for commercial licensing options. +*/ +#ifndef KDSINGLEAPPLICATION_LIB_H +#define KDSINGLEAPPLICATION_LIB_H + +#include + +#if defined(KDSINGLEAPPLICATION_STATIC_BUILD) +# define KDSINGLEAPPLICATION_EXPORT +#elif defined(KDSINGLEAPPLICATION_SHARED_BUILD) +# define KDSINGLEAPPLICATION_EXPORT Q_DECL_EXPORT +#else +# define KDSINGLEAPPLICATION_EXPORT Q_DECL_IMPORT +#endif + +#endif // KDSINGLEAPPLICATION_LIB_H diff --git a/src/main.cpp b/src/main.cpp index 02a9236..310c29b 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1,27 +1,50 @@ #include "application.h" +#include "db/sqltimesheetrepository.h" +#include "exceptions.h" #include "kdsingleapplication/kdsingleapplication.h" #include "mainwindow.h" +#include "timesheet.h" #include +#include int main(int argc, char** argv) { Application app(argc, argv); - KDSingleApplication kdsa; - MainWindow w; - if (kdsa.isPrimaryInstance()) { - QObject::connect(&kdsa, &KDSingleApplication::messageReceived, &app.qapp(), [&w](const QByteArray&) { - w.setWindowState((w.windowState() & ~Qt::WindowMinimized) | Qt::WindowActive); - w.raise(); // for MacOS - w.activateWindow(); // for Windows - }); - if (const auto& filename = Application::timesheet_filename(); !filename.empty()) { - w.load(filename); - } - w.show(); - } else { + + // The single-instance lock is named after the database, so the invariant is one process per + // database rather than one process overall -- otherwise a --database override would be + // swallowed whenever the regular instance happens to be running. + KDSingleApplication kdsa{Application::single_instance_name()}; + if (!kdsa.isPrimaryInstance()) { kdsa.sendMessage({}); return 0; } - QApplication::exec(); + // Only the primary instance touches the database, hence the check above this point. + if (const auto result = app.open_database(); !result.ok) { + QMessageBox::critical(nullptr, QApplication::applicationDisplayName(), result.message); + return 1; + } + + auto time_sheet = std::unique_ptr{}; + try { + time_sheet = Application::sql_repository().load(); + } catch (const DatabaseError& e) { + QMessageBox::critical( + nullptr, QApplication::applicationDisplayName(), + QObject::tr("Cannot read the database '%1': %2") + .arg(QString::fromStdString(Application::database_path().string()), QString::fromStdString(e.what()))); + return 1; + } + + // The window is constructed with its data, so the views are never wired to a placeholder. + MainWindow w{std::move(time_sheet)}; + QObject::connect(&kdsa, &KDSingleApplication::messageReceived, &app.qapp(), [&w](const QByteArray&) { + w.setWindowState((w.windowState() & ~Qt::WindowMinimized) | Qt::WindowActive); + w.raise(); // for MacOS + w.activateWindow(); // for Windows + }); + w.show(); + + return QApplication::exec(); } diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 7cdd83b..240c6d1 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -7,47 +7,41 @@ #include "exceptions.h" #include "intervalmodel.h" #include "plan.h" +#include "plansettingsdialog.h" #include "projectmodel.h" -#include "serialization.h" #include "timesheet.h" #include "ui_mainwindow.h" #include -#include +#include #include +#include #include -#include -#include #include -namespace +MainWindow::MainWindow(std::unique_ptr time_sheet) + : m_ui(std::make_unique()), m_view_action_group(this) { + m_ui->setupUi(this); + set_time_sheet(std::move(time_sheet)); -constexpr auto extension = ".ts"; -[[nodiscard]] auto file_filter() -{ - return QObject::tr("Time Sheets (*%1)").arg(extension); -} - -} // namespace + m_persistence_status_label = new QLabel{this}; + m_persistence_status_label->setStyleSheet(QStringLiteral("QLabel { color: red; }")); + m_persistence_status_label->hide(); + // A permanent widget, not showMessage(): the status bar's transient slot is used for the + // period label and would overwrite a warning on the next navigation keystroke. + statusBar()->addPermanentWidget(m_persistence_status_label); + connect(&Application::undo_stack(), &UndoStack::write_failed, this, &MainWindow::on_write_failed); + connect(&Application::undo_stack(), &UndoStack::write_succeeded, this, &MainWindow::on_write_succeeded); -MainWindow::MainWindow(QWidget* parent) - : QMainWindow(parent) - , m_ui(std::make_unique()) - , m_time_sheet(std::make_unique()) - , m_view_action_group(this) -{ - m_ui->setupUi(this); m_ui->period_detail_view->setContextMenuPolicy(Qt::CustomContextMenu); connect(m_ui->period_detail_view, &PeriodDetailView::current_interval_changed, m_ui->ganttview, &GanttView::set_current_interval); connect(m_ui->period_detail_view, &PeriodDetailView::period_changed, m_ui->ganttview, &GanttView::ensure_visible); connect(m_ui->ganttview, &GanttView::clicked, this, [this](const QDateTime& timestamp) { set_period(Period{timestamp.date(), Period::Type::Day}); }); - connect(m_ui->action_Load, &QAction::triggered, this, QOverload<>::of(&MainWindow::load)); - connect(m_ui->action_Save, &QAction::triggered, this, &MainWindow::save); - connect(m_ui->action_Save_As, &QAction::triggered, this, &MainWindow::save_as); - connect(m_ui->action_New_time_sheet, &QAction::triggered, this, &MainWindow::new_time_sheet); + connect(m_ui->actionQuit, &QAction::triggered, this, &QMainWindow::close); + connect(m_ui->actionPlan_Settings, &QAction::triggered, this, &MainWindow::edit_plan_settings); connect(m_ui->action_Add_Interval, &QAction::triggered, this, [this]() { auto interval = std::make_unique(nullptr); @@ -81,23 +75,33 @@ MainWindow::MainWindow(QWidget* parent) init_view_action(m_ui->actionMonth, Period::Type::Month); init_view_action(m_ui->actionWeek, Period::Type::Week); init_view_action(m_ui->actionDay, Period::Type::Day); - m_ui->actionDay->trigger(); connect(m_ui->actionNext, &QAction::triggered, this, &MainWindow::next); connect(m_ui->actionPrevious, &QAction::triggered, this, &MainWindow::previous); connect(m_ui->actionToday, &QAction::triggered, this, &MainWindow::today); - auto* const undo_action = Application::undo_stack().impl().createUndoAction(this); - m_ui->menu_Edit->addAction(undo_action); + // Built by hand rather than with QUndoStack::createUndoAction, because those trigger + // QUndoStack::undo()/redo() directly and would bypass UndoStack's transaction handling -- + // undoing a forty-interval macro would then run forty separate transactions. + auto* const undo_action = new QAction{tr("&Undo"), this}; undo_action->setShortcut(QKeySequence(Qt::CTRL | Qt::Key_Z)); - auto* const redo_action = Application::undo_stack().impl().createRedoAction(this); + undo_action->setEnabled(Application::undo_stack().impl().canUndo()); + connect(undo_action, &QAction::triggered, this, [] { Application::undo_stack().undo(); }); + connect(&Application::undo_stack().impl(), &QUndoStack::canUndoChanged, undo_action, &QAction::setEnabled); + m_ui->menu_Edit->addAction(undo_action); + + auto* const redo_action = new QAction{tr("&Redo"), this}; redo_action->setShortcut(QKeySequence(Qt::CTRL | Qt::Key_Y)); + redo_action->setEnabled(Application::undo_stack().impl().canRedo()); + connect(redo_action, &QAction::triggered, this, [] { Application::undo_stack().redo(); }); + connect(&Application::undo_stack().impl(), &QUndoStack::canRedoChanged, redo_action, &QAction::setEnabled); m_ui->menu_Edit->addAction(redo_action); - connect(&Application::undo_stack().impl(), &QUndoStack::cleanChanged, this, - [this](const bool clean) { setWindowModified(!clean); }); - - new_time_sheet(); + update_window_title(); + // Establishing the period must come last: set_period reads Plan::start(), so it needs the + // loaded plan, and it pushes the period into views that must already have their models. + m_ui->actionDay->trigger(); + today(); } MainWindow::~MainWindow() = default; @@ -111,13 +115,6 @@ void MainWindow::set_time_sheet(std::unique_ptr time_sheet) m_ui->ganttview->set_time_sheet(m_time_sheet.get()); m_ui->tv_plan->setModel(&m_time_sheet->plan()); connect(&m_time_sheet->plan(), &Plan::plan_changed, m_ui->plan_view, &PlanView::invalidate); - Application::undo_stack().impl().clear(); -} - -void MainWindow::set_filename(std::filesystem::path filename) -{ - m_filename = std::move(filename); - update_window_title(); } void MainWindow::end_task() @@ -163,7 +160,6 @@ void MainWindow::switch_task() void MainWindow::update_window_title() { - const auto filename_part = m_filename.empty() ? tr("Untitled") : QString::fromStdString(m_filename.string()); static const auto app_now_hint = [] { if (const auto app_now = Application::current_date_time(); app_now != QDateTime::currentDateTime()) { spdlog::warn("Application now ({}) doesn't match system now ({}). This may be useful for debugging only.", @@ -172,96 +168,59 @@ void MainWindow::update_window_title() } return QStringLiteral(); }(); - QStringList title{tr("%1[*] — %2").arg(filename_part, QApplication::applicationDisplayName())}; + QStringList title{QApplication::applicationDisplayName()}; if (!app_now_hint.isEmpty()) { title.append(app_now_hint); } - setWindowTitle(title.join(" ")); -} - -bool MainWindow::can_close() -{ - if (Application::undo_stack().impl().isClean()) { - return true; + // The default database needs no mention; an override does, so it is never unclear which records + // are being edited. + if (!Application::is_default_database()) { + title.append(tr("DB=%1").arg(QString::fromStdString(Application::database_path().string()))); } - - const auto answer = QMessageBox::question(this, QApplication::applicationDisplayName(), - tr("Do you want to save pending changes before close?"), - QMessageBox::Save | QMessageBox::Discard | QMessageBox::Abort); - return answer == QMessageBox::Discard || (answer == QMessageBox::Save && save()); + setWindowTitle(title.join(" ")); } -bool MainWindow::load() +void MainWindow::on_write_failed(const QString& message) { - const auto last_load_dir = QDir::home().path(); // TODO - const auto q_filename = - QFileDialog::getOpenFileName(this, QApplication::applicationDisplayName(), last_load_dir, file_filter()); - if (q_filename.isEmpty()) { - return false; + m_persistence_status_label->setText(tr("Not saving — changes are only in memory")); + m_persistence_status_label->show(); + if (m_persistence_failed) { + // Already reported. A broken database fails on every keystroke, and one modal per failure + // would be an unclosable dialog storm. + return; } - - return load(static_cast(q_filename.toStdString())); + m_persistence_failed = true; + QMessageBox::critical(this, QApplication::applicationDisplayName(), + tr("Cannot write to the database '%1':\n\n%2\n\nYour recent changes are still shown but " + "are not saved. Copy anything you cannot lose before closing.") + .arg(QString::fromStdString(Application::database_path().string()), message)); } -bool MainWindow::load(std::filesystem::path filename) +void MainWindow::on_write_succeeded() { - if (!can_close()) { - return false; - } - - try { - std::ifstream ifs(filename); - if (!ifs) { - QMessageBox::critical(this, QApplication::applicationDisplayName(), - tr("Failed to open '%1' for reading.").arg(QString::fromStdString(m_filename.string()))); - } - nlohmann::json data; - ifs >> data; - set_time_sheet(::deserialize(data)); - set_filename(std::move(filename)); - return true; - } catch (const DeserializationError& e) { - QMessageBox::critical( - this, QApplication::applicationDisplayName(), - tr("Failed to open '%1': %2").arg(QString::fromStdString(filename.string()), QString::fromStdString(e.what()))); - } catch (const nlohmann::json::parse_error& e) { - QMessageBox::critical( - this, QApplication::applicationDisplayName(), - tr("Failed to open '%1': %2").arg(QString::fromStdString(filename.string()), QString::fromStdString(e.what()))); + if (!m_persistence_failed) { + return; } - return false; + m_persistence_failed = false; + m_persistence_status_label->hide(); } -bool MainWindow::save() +void MainWindow::edit_plan_settings() { - if (m_filename.empty()) { - return save_as(); - } - - std::ofstream ofs(m_filename); - if (!ofs) { - QMessageBox::critical(this, QApplication::applicationDisplayName(), - tr("Failed to open '%1' for writing.").arg(QString::fromStdString(m_filename.string()))); - return false; - } - ofs << ::serialize(*m_time_sheet); - Application::undo_stack().impl().setClean(); - return true; + PlanSettingsDialog dialog{m_time_sheet->plan(), this}; + dialog.exec(); } -bool MainWindow::save_as() +bool MainWindow::can_close() { - const auto last_load_dir = QDir::home().path(); // TODO - const auto q_filename = - QFileDialog::getSaveFileName(this, QApplication::applicationDisplayName(), last_load_dir, file_filter()); - - if (q_filename.isEmpty()) { - return false; + if (!m_persistence_failed) { + return true; } - set_filename(static_cast(q_filename.toStdString())); - save(); - Application::undo_stack().impl().setClean(); - return true; + const auto answer = + QMessageBox::warning(this, QApplication::applicationDisplayName(), + tr("Recent changes could not be written to the database and will be lost. Quit anyway?"), + QMessageBox::Discard | QMessageBox::Cancel); + return answer == QMessageBox::Discard; } void MainWindow::closeEvent(QCloseEvent* event) @@ -269,17 +228,6 @@ void MainWindow::closeEvent(QCloseEvent* event) can_close() ? event->accept() : event->ignore(); } -bool MainWindow::new_time_sheet() -{ - if (!can_close()) { - return false; - } - set_time_sheet(std::make_unique()); - set_filename({}); - set_date(Application::current_date_time().date()); - return true; -} - void MainWindow::next() { set_date(m_current_period.end().addDays(1)); diff --git a/src/mainwindow.h b/src/mainwindow.h index 79eb8dc..714b150 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -5,10 +5,10 @@ #include "period.h" #include #include -#include #include #include +class QLabel; class TimeSheet; class UndoStack; @@ -22,16 +22,13 @@ class MainWindow : public QMainWindow Q_OBJECT public: - explicit MainWindow(QWidget* parent = nullptr); + /** + * @brief Takes the timesheet the window will display. + * Requiring the data up front makes "the models exist before the views are wired" a property of + * the type rather than a call-order convention. + */ + explicit MainWindow(std::unique_ptr time_sheet); ~MainWindow() override; - void set_time_sheet(std::unique_ptr time_sheet); - void set_filename(std::filesystem::path filename); - - bool load(); - bool load(std::filesystem::path filename); - bool save(); - bool save_as(); - bool new_time_sheet(); void next(); void previous(); @@ -49,13 +46,28 @@ class MainWindow : public QMainWindow private: std::unique_ptr m_ui; std::unique_ptr m_time_sheet; - std::filesystem::path m_filename; QActionGroup m_view_action_group; + void set_time_sheet(std::unique_ptr time_sheet); void end_task(); void switch_task(); + void edit_plan_settings(); void update_window_title(); + /** + * @name Persistence failure reporting + * A silent write failure would be data loss, so a failure is shown three ways: logged, made + * permanently visible in the status bar, and raised once in a dialog. + * @{ + */ + void on_write_failed(const QString& message); + void on_write_succeeded(); + /** @} */ + + /** @brief Only blocks closing while writes are failing; there is nothing to save otherwise. */ [[nodiscard]] bool can_close(); + Period m_current_period; + QLabel* m_persistence_status_label = nullptr; + bool m_persistence_failed = false; }; diff --git a/src/mainwindow.ui b/src/mainwindow.ui index 824f245..3afad4a 100644 --- a/src/mainwindow.ui +++ b/src/mainwindow.ui @@ -134,11 +134,7 @@ &File - - - - - + @@ -187,30 +183,6 @@ - - - &Load ... - - - Ctrl+O - - - - - &Save As ... - - - Ctrl+Shift+S - - - - - &Save - - - Ctrl+S - - &Quit @@ -295,9 +267,9 @@ Ctrl+Space - + - &New time sheet + &Plan Settings ... diff --git a/src/period.cpp b/src/period.cpp index 61a80c3..66bfdf7 100644 --- a/src/period.cpp +++ b/src/period.cpp @@ -1,9 +1,7 @@ #include "period.h" -#include "enum.h" #include "fmt.h" #include "interval.h" -#include "json.h" #include #include #include @@ -223,37 +221,3 @@ std::vector Period::dates() const } return days; } - -void to_json(nlohmann::json& j, const Period& value) -{ - if (value.type() == Period::Type::Custom) { - j = { - {begin_key, value.begin()}, - {end_key, value.end()}, - }; - } else { - j = { - {begin_key, value.begin()}, - {type_key, value.type()}, - }; - } -} - -void from_json(const nlohmann::json& j, Period& value) -{ - if (const auto it = j.find(type_key); it != j.end()) { - value = Period(j.at(begin_key), static_cast(*it)); - } else { - value = Period(j.at(begin_key), static_cast(j.at(end_key))); - } -} - -void to_json(nlohmann::json& j, const Period::Type& value) -{ - j = fmt::format("{}", value); -} - -void from_json(const nlohmann::json& j, Period::Type& value) -{ - value = ::enum_from_string(j); -} diff --git a/src/period.h b/src/period.h index d2ae2b4..74a7e94 100644 --- a/src/period.h +++ b/src/period.h @@ -2,7 +2,6 @@ #include #include -#include class Interval; @@ -59,8 +58,3 @@ template<> struct fmt::formatter : fmt::formatter using format_return_type = decltype(std::declval().out()); [[nodiscard]] format_return_type format(const Period::Type& t, fmt::format_context& ctx) const; }; - -void to_json(nlohmann::json& j, const Period& value); -void from_json(const nlohmann::json& j, Period& value); -void to_json(nlohmann::json& j, const Period::Type& value); -void from_json(const nlohmann::json& j, Period::Type& value); diff --git a/src/plan.cpp b/src/plan.cpp index 3e492ee..0dcaa89 100644 --- a/src/plan.cpp +++ b/src/plan.cpp @@ -1,24 +1,17 @@ #include "plan.h" -#include "enum.h" +#include "db/abstracttimesheetrepository.h" +#include "exceptions.h" #include "intervalmodel.h" -#include "json.h" #include "period.h" #include "periodedit.h" #include #include -#include #include namespace { -constexpr auto start_key = "start"; -constexpr auto overtime_offset_key = "overtime_offset"; -constexpr auto periods_key = "periods"; -constexpr auto period_key = "period"; -constexpr auto kind_key = "kind"; - struct SickLeaveFactors { using enum Plan::Kind; @@ -73,49 +66,24 @@ struct HolidayLeaveFactors } // namespace -template<> struct nlohmann::adl_serializer> -{ - static void to_json(json& json_value, const std::unique_ptr& ptr) - { - if (ptr == nullptr) { - json_value = nullptr; - } else { - json_value = *ptr; - } - } - - static void from_json(const json& json_value, std::unique_ptr& ptr) - { - if (json_value.is_null()) { - ptr = {}; - } else { - ptr = std::make_unique(static_cast(json_value)); - } - } -}; - -Plan::Plan(const nlohmann::json& data) : m_start(data.at(start_key)), m_overtime_offset(data.at(overtime_offset_key)) +Plan::Plan() : Plan(null_repository()) { - if (data.contains(periods_key)) { - m_periods = data.at(periods_key); - } - sort(); - if (!is_sorted()) { - throw RuntimeError("Failed to sort periods in plan: overlapping periods cannot be sorted."); - } } -Plan::Plan() +Plan::Plan(AbstractTimeSheetRepository& repository) : m_repository(repository) { } -nlohmann::json Plan::to_json() const noexcept +Plan::Plan(AbstractTimeSheetRepository& repository, const QDate& start, const std::chrono::minutes overtime_offset, + std::vector> entries) + : m_repository(repository), m_start(start), m_overtime_offset(overtime_offset), m_periods(std::move(entries)) { - return { - {start_key, m_start}, - {overtime_offset_key, m_overtime_offset}, - {periods_key, m_periods}, - }; + // Deliberately not going through add(): these entries are already stored, and add() would + // write every one of them straight back. + sort(); + if (!is_sorted()) { + throw DatabaseError("The stored plan contains overlapping periods and cannot be sorted."); + } } std::chrono::minutes Plan::planned_working_time(const QDate& date, const Kind kind, @@ -211,7 +179,10 @@ Period Plan::default_period() const noexcept { const auto date = Application::current_date_time().date(); const Period candidate{date, Period::Type::Day}; - if (find_period_insert_pos(m_periods, candidate) == m_periods.end()) { + // has_value(), not "== end()": find_period_insert_pos returns an optional, and comparing it to + // end() asked for "insertable *and* last", so today was skipped whenever any later entry + // existed. + if (find_period_insert_pos(m_periods, candidate).has_value()) { return candidate; } return Period{m_periods.back()->period.end().addDays(1), Period::Type::Day}; @@ -236,12 +207,12 @@ QVariant Plan::data(const QModelIndex& index, const int role) const return {}; } - const auto& [period, kind] = *m_periods.at(index.row()); + const auto& entry = *m_periods.at(index.row()); switch (index.column()) { case period_column: - return period.label(); + return entry.period.label(); case kind_column: - return QString::fromStdString(fmt::format("{}", kind)); + return QString::fromStdString(fmt::format("{}", entry.kind)); default: Q_UNREACHABLE(); } @@ -288,15 +259,19 @@ bool Plan::add(std::unique_ptr entry) { const auto insert_pos = find_period_insert_pos(m_periods, entry->period); if (!insert_pos.has_value()) { - spdlog::warn("Failed to find insert position for {} in {} because it overlaps with the existing periods.", - entry->period, m_periods | std::views::transform([](const auto& e) { return e->period; })); + // Caution: returning false here destroys `entry`. An AddCommand that hit this would be left + // holding a dangling reference, so callers must ensure the period is free first -- see + // can_set_period() and default_period(). Kept loud rather than silent for that reason. + spdlog::error("Refusing to add {} because it overlaps the existing periods {}. The entry is dropped.", + entry->period, m_periods | std::views::transform([](const auto& e) { return e->period; })); return false; } const auto row = std::distance(m_periods.cbegin(), *insert_pos); beginInsertRows({}, row, row); - m_periods.insert(*insert_pos, std::move(entry)); + auto& ref = **m_periods.insert(*insert_pos, std::move(entry)); endInsertRows(); + m_repository.insert(ref); Q_EMIT plan_changed(); assert(is_sorted()); return true; @@ -308,6 +283,9 @@ std::unique_ptr Plan::extract(const Entry& entry) std::ranges::find_if(m_periods, [&entry](const auto& candidate) { return candidate.get() == &entry; }); it != m_periods.end()) { + // Delete the row before touching the container, so a failure leaves the plan untouched rather + // than destroying the entry during unwinding and dangling the calling command's reference. + m_repository.remove(entry); const auto row = std::distance(m_periods.begin(), it); beginRemoveRows({}, row, row); auto ret = std::move(*it); @@ -357,24 +335,81 @@ std::chrono::minutes Plan::planned_normal_working_time(const Period& period) con return result; } -void Plan::set_data(const int row, Kind kind) +QDate Plan::swap_start(QDate start) +{ + using std::swap; + swap(m_start, start); + m_repository.update_plan_setting(*this); + Q_EMIT plan_changed(); + return start; +} + +std::chrono::minutes Plan::swap_overtime_offset(std::chrono::minutes overtime_offset) +{ + using std::swap; + swap(m_overtime_offset, overtime_offset); + m_repository.update_plan_setting(*this); + Q_EMIT plan_changed(); + return overtime_offset; +} + +Plan::Entry& Plan::find_entry(const Entry& entry) +{ + const auto it = std::ranges::find(m_periods, &entry, [](const auto& candidate) { return candidate.get(); }); + if (it == m_periods.end()) { + throw RuntimeError("The entry does not belong to this plan."); + } + return **it; +} + +int Plan::row_of(const Entry& entry) const +{ + const auto it = std::ranges::find(m_periods, &entry, [](const auto& candidate) { return candidate.get(); }); + return static_cast(std::distance(m_periods.begin(), it)); +} + +Plan::Kind Plan::swap_kind(const Entry& entry_ref, Kind kind) { using std::swap; - swap(m_periods.at(row)->kind, kind); - data_changed(row, kind_column); + auto& entry = find_entry(entry_ref); + swap(entry.kind, kind); + m_repository.update(entry); + data_changed(row_of(entry), kind_column); + return kind; } -void Plan::set_data(const int row, Period period) +Period Plan::swap_period(const Entry& entry_ref, Period period) { - Period& old_period = m_periods.at(row)->period; - swap(old_period, period); + auto& entry = find_entry(entry_ref); + swap(entry.period, period); + // A new period generally belongs at a different row, so this is a reordering, not a cell edit. + // Announcing it as a reset keeps the view and any persistent indices honest -- emitting only + // dataChanged for one cell would leave every other row displaying the wrong entry. + beginResetModel(); sort(); - if (!is_sorted()) { - swap(old_period, period); + const auto sorted = is_sorted(); + if (!sorted) { + // Put it back before anyone observes the broken ordering, and persist nothing. + swap(entry.period, period); sort(); + } + endResetModel(); + if (!sorted) { throw RuntimeError("Failed to change period because it would overlap."); } - data_changed(row, period_column); + m_repository.update(entry); + Q_EMIT plan_changed(); + return period; +} + +bool Plan::can_set_period(const Entry& entry, const Period& period) const +{ + // The entry being moved is excluded, because it may of course overlap its own current period. + // Period::overlap treats touching periods (one's end equal to the next one's begin) as + // overlapping, which is the same rule is_sorted() enforces, so this agrees with swap_period(). + const auto is_other = [&entry](const auto& candidate) { return candidate.get() != &entry; }; + const auto overlaps = [&period](const auto& candidate) { return candidate->period.overlap(period).has_value(); }; + return std::ranges::none_of(m_periods | std::views::filter(is_other), overlaps); } std::chrono::minutes Plan::sick_time(const Period& period) const @@ -400,30 +435,6 @@ std::chrono::minutes FullTimePlan::planned_normal_working_time(const QDate& date return day == Qt::Saturday || day == Qt::Sunday ? 0min : 8h; } -void to_json(nlohmann::json& j, const Plan::Entry& value) -{ - j = { - {period_key, value.period}, - {kind_key, value.kind}, - }; -} - -void from_json(const nlohmann::json& j, Plan::Entry& value) -{ - value.period = j.at(period_key); - value.kind = j.at(kind_key); -} - -void to_json(nlohmann::json& j, const Plan::Kind& value) -{ - j = fmt::format("{}", value); -} - -void from_json(const nlohmann::json& j, Plan::Kind& value) -{ - value = ::enum_from_string(j); -} - [[nodiscard]] bool Plan::is_sorted() const noexcept { return m_periods.empty() diff --git a/src/plan.h b/src/plan.h index 5889412..8c85a03 100644 --- a/src/plan.h +++ b/src/plan.h @@ -1,6 +1,7 @@ #pragma once #include "application.h" +#include "db/entityid.h" #include "fmt.h" #include "period.h" @@ -8,6 +9,7 @@ #include #include +class AbstractTimeSheetRepository; class IntervalModel; class QDate; @@ -17,15 +19,43 @@ class Plan : public QAbstractTableModel public: static constexpr auto period_column = 0; static constexpr auto kind_column = 1; - explicit Plan(const nlohmann::json& data); + + enum class Kind { Normal, Sick, Holiday, HalfHoliday, Vacation, HalfVacation, HalfVacationHalfHoliday }; + + struct Entry + { + Period period; + Kind kind; + /** + * @brief Identity of this entry's row, invalid until it has been persisted. + * Deliberately the last member and deliberately public: Entry is an aggregate, initialized + * both positionally and with designated initializers, and adding a base class or a private + * member would break those call sites. + */ + EntityId id; + }; + + /** @brief Creates a plan that does not persist anything. */ explicit Plan(); - [[nodiscard]] nlohmann::json to_json() const noexcept; + explicit Plan(AbstractTimeSheetRepository& repository); + /** @brief Adopts already-stored entries without writing them back. */ + explicit Plan(AbstractTimeSheetRepository& repository, const QDate& start, std::chrono::minutes overtime_offset, + std::vector> entries); [[nodiscard]] std::chrono::minutes planned_working_time(const Period& period, const IntervalModel& interval_model) const; [[nodiscard]] const std::chrono::minutes& overtime_offset() const noexcept; [[nodiscard]] const QDate& start() const noexcept; - enum class Kind { Normal, Sick, Holiday, HalfHoliday, Vacation, HalfVacation, HalfVacationHalfHoliday }; + /** + * @name Plan settings + * The date overtime accounting starts from, and a manual correction added to the balance. + * Both swap in the new value, persist, and return the previous one. + * @{ + */ + QDate swap_start(QDate start); + std::chrono::minutes swap_overtime_offset(std::chrono::minutes overtime_offset); + /** @} */ + [[nodiscard]] Kind find_kind(const QDate& date) const; /** @@ -40,18 +70,31 @@ class Plan : public QAbstractTableModel [[nodiscard]] QVariant headerData(int section, Qt::Orientation orientation, int role) const override; [[nodiscard]] Qt::ItemFlags flags(const QModelIndex& index) const override; - struct Entry - { - Period period; - Kind kind; - }; - bool add(std::unique_ptr entry); std::unique_ptr extract(const Entry& entry); const Entry& entry(int row) const noexcept; - void set_data(int row, Kind kind); - void set_data(int row, Period period); + + /** + * @name Mutators + * Each swaps in a new value, persists the entry and returns the previous value, so it can serve + * as its own inverse in a ModifyCommand. + * + * They identify the entry by reference rather than by row on purpose: changing a period + * re-sorts the entries, so a row index captured before the change refers to a different entry + * afterwards -- and an undo keyed on it would silently rewrite the wrong one. + * @{ + */ + Kind swap_kind(const Entry& entry, Kind kind); + /** @throws RuntimeError if @p period would overlap another entry. See can_set_period(). */ + Period swap_period(const Entry& entry, Period period); + /** @} */ + + /** + * @brief Whether swap_period() would succeed. + * Lets a caller check before pushing an undo command, rather than having one throw from redo(). + */ + [[nodiscard]] bool can_set_period(const Entry& entry, const Period& period) const; [[nodiscard]] std::chrono::minutes sick_time(const Period& period) const; [[nodiscard]] std::chrono::minutes holiday_time(const Period& period) const; @@ -65,10 +108,14 @@ class Plan : public QAbstractTableModel [[nodiscard]] virtual std::chrono::minutes planned_normal_working_time(const QDate& date) const noexcept = 0; private: + AbstractTimeSheetRepository& m_repository; QDate m_start = Application::current_date_time().date(); std::chrono::minutes m_overtime_offset{0}; std::vector> m_periods; void data_changed(int row, int column); + /** @throws RuntimeError if @p entry does not belong to this plan. */ + [[nodiscard]] Entry& find_entry(const Entry& entry); + [[nodiscard]] int row_of(const Entry& entry) const; template [[nodiscard]] std::chrono::minutes count(const Period& period) const; [[nodiscard]] std::chrono::minutes planned_normal_working_time(const Period& period) const noexcept; [[nodiscard]] std::chrono::minutes planned_working_time(const QDate& date, Kind kind, @@ -124,10 +171,5 @@ template<> struct fmt::formatter : formatter } }; -void to_json(nlohmann::json& j, const Plan::Entry& value); -void from_json(const nlohmann::json& j, Plan::Entry& value); -void to_json(nlohmann::json& j, const Plan::Kind& value); -void from_json(const nlohmann::json& j, Plan::Kind& value); - std::optional>::const_iterator> find_period_insert_pos(const std::vector>& periods, const Period& candidate) noexcept; diff --git a/src/plansettingsdialog.cpp b/src/plansettingsdialog.cpp new file mode 100644 index 0000000..123dd95 --- /dev/null +++ b/src/plansettingsdialog.cpp @@ -0,0 +1,65 @@ +#include "plansettingsdialog.h" + +#include "application.h" +#include "commands/commands.h" +#include "commands/undostack.h" +#include "plan.h" + +#include +#include +#include +#include + +namespace +{ + +// Wide enough for a few working years of accumulated correction in either direction. +constexpr auto overtime_offset_limit_minutes = 1'000'000; + +} // namespace + +PlanSettingsDialog::PlanSettingsDialog(Plan& plan, QWidget* parent) : QDialog(parent), m_plan(plan) +{ + setWindowTitle(tr("Plan Settings")); + + m_start_edit = new QDateEdit{m_plan.start(), this}; + m_start_edit->setCalendarPopup(true); + m_start_edit->setToolTip(tr("Overtime is accounted for from this date on.")); + + m_overtime_offset_edit = new QSpinBox{this}; + m_overtime_offset_edit->setRange(-overtime_offset_limit_minutes, overtime_offset_limit_minutes); + m_overtime_offset_edit->setSuffix(tr(" min")); + m_overtime_offset_edit->setValue(static_cast(m_plan.overtime_offset().count())); + m_overtime_offset_edit->setToolTip(tr("A correction added to the computed overtime balance. May be negative.")); + + auto* const buttons = new QDialogButtonBox{QDialogButtonBox::Ok | QDialogButtonBox::Cancel, this}; + connect(buttons, &QDialogButtonBox::accepted, this, &QDialog::accept); + connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject); + + auto* const layout = new QFormLayout{this}; + layout->addRow(tr("Plan &start:"), m_start_edit); + layout->addRow(tr("&Overtime offset:"), m_overtime_offset_edit); + layout->addWidget(buttons); +} + +PlanSettingsDialog::~PlanSettingsDialog() = default; + +void PlanSettingsDialog::accept() +{ + const auto start = m_start_edit->date(); + const auto overtime_offset = std::chrono::minutes{m_overtime_offset_edit->value()}; + const auto start_changed = start != m_plan.start(); + const auto offset_changed = overtime_offset != m_plan.overtime_offset(); + + if (start_changed || offset_changed) { + // One macro, so changing both is a single undo step and a single transaction. + const auto macro = Application::undo_stack().start_macro(tr("Change plan settings")); + if (start_changed) { + Application::undo_stack().push(make_modify_plan_start_command(m_plan, start)); + } + if (offset_changed) { + Application::undo_stack().push(make_modify_plan_overtime_offset_command(m_plan, overtime_offset)); + } + } + QDialog::accept(); +} diff --git a/src/plansettingsdialog.h b/src/plansettingsdialog.h new file mode 100644 index 0000000..dd3b138 --- /dev/null +++ b/src/plansettingsdialog.h @@ -0,0 +1,31 @@ +#pragma once + +#include +#include + +class Plan; +class QDateEdit; +class QSpinBox; + +/** + * @class PlanSettingsDialog plansettingsdialog.h "plansettingsdialog.h" + * @brief Edits the plan's start date and overtime offset. + * + * These two values used to be reachable only by hand-editing the JSON timesheet. With the store + * moved to a database that is no longer possible, so they get a proper dialog. Both edits go + * through the undo stack, and therefore through the same write-through path as everything else. + */ +class PlanSettingsDialog : public QDialog +{ + Q_OBJECT +public: + explicit PlanSettingsDialog(Plan& plan, QWidget* parent); + ~PlanSettingsDialog() override; + + void accept() override; + +private: + Plan& m_plan; + QDateEdit* m_start_edit = nullptr; + QSpinBox* m_overtime_offset_edit = nullptr; +}; diff --git a/src/plantableview.cpp b/src/plantableview.cpp index b4cc8a5..0d21df6 100644 --- a/src/plantableview.cpp +++ b/src/plantableview.cpp @@ -1,5 +1,8 @@ #include "plantableview.h" +#include "application.h" +#include "commands/commands.h" +#include "commands/undostack.h" #include "periodedit.h" #include "plan.h" @@ -30,7 +33,13 @@ class KindDelegate : public QStyledItemDelegate void setModelData(QWidget* const editor, QAbstractItemModel* const model, const QModelIndex& index) const override { - dynamic_cast(*model).set_data(index.row(), dynamic_cast(*editor).current_enum()); + auto& plan = dynamic_cast(*model); + const auto& entry = plan.entry(index.row()); + const auto kind = dynamic_cast(*editor).current_enum(); + if (kind == entry.kind) { + return; + } + Application::undo_stack().push(make_modify_plan_kind_command(plan, entry, kind)); } }; @@ -49,12 +58,17 @@ void PlanTableView::open_period_edit(const QModelIndex& index) { PeriodEdit period_edit; auto& plan = dynamic_cast(*model()); - period_edit.set_period(plan.entry(index.row()).period); - if (period_edit.exec() == QDialog::Accepted) { - try { - plan.set_data(index.row(), period_edit.period()); - } catch (const RuntimeError& e) { - QMessageBox::critical(this, tr("Failed to set period"), e.what()); - } + const auto& entry = plan.entry(index.row()); + period_edit.set_period(entry.period); + if (period_edit.exec() != QDialog::Accepted) { + return; + } + const auto period = period_edit.period(); + // Validate up front: pushing a command whose redo() throws would leave the undo stack holding + // an entry that never applied. + if (!plan.can_set_period(entry, period)) { + QMessageBox::critical(this, tr("Failed to set period"), tr("The period would overlap another one.")); + return; } + Application::undo_stack().push(make_modify_plan_period_command(plan, entry, period)); } diff --git a/src/project.cpp b/src/project.cpp index da31b3c..5c3cb96 100644 --- a/src/project.cpp +++ b/src/project.cpp @@ -2,20 +2,6 @@ #include "exceptions.h" #include "fmt.h" #include "period.h" -#include - -namespace -{ - -constexpr auto name_key = "name"; -constexpr auto color_key = "color"; - -} // namespace - -Project::Project(const nlohmann::json& data) - : m_name(data.at(name_key)), m_color(QColor::fromString(data.value(color_key, QString{}))) -{ -} Project::Project(QString name, const QColor& color) : m_name(std::move(name)), m_color(color) { @@ -26,14 +12,6 @@ const QString& Project::name() const noexcept return m_name; } -nlohmann::json Project::to_json() const -{ - return { - {name_key, m_name}, - {color_key, m_color.name()}, - }; -} - const QColor& Project::color() const noexcept { return m_color; @@ -44,6 +22,16 @@ void Project::set_color(const QColor& color) noexcept m_color = color; } +EntityId Project::id() const noexcept +{ + return m_id; +} + +void Project::set_id(const EntityId id) noexcept +{ + m_id = id; +} + fmt::formatter::format_return_type fmt::formatter::format(const Project& p, fmt::format_context& ctx) { return fmt::format_to(ctx.out(), "Project[{}]", p.name().toStdString()); diff --git a/src/project.h b/src/project.h index aa5939a..a8064c8 100644 --- a/src/project.h +++ b/src/project.h @@ -1,25 +1,32 @@ #pragma once +#include "db/entityid.h" #include "fmt.h" -#include "json.h" #include class Project { public: - explicit Project(const nlohmann::json& data); explicit Project(QString name, const QColor& color); explicit Project() = default; [[nodiscard]] const QString& name() const noexcept; - [[nodiscard]] nlohmann::json to_json() const; [[nodiscard]] const QColor& color() const noexcept; void set_color(const QColor& color) noexcept; + /** + * @brief Identity of this project's row, invalid until it has been persisted. + * The id travels with the object so that a project removed by a command and put back by undo + * returns to its original row, keeping the intervals that reference it valid. + */ + [[nodiscard]] EntityId id() const noexcept; + void set_id(EntityId id) noexcept; + private: QString m_name; QColor m_color; + EntityId m_id; }; template<> struct fmt::formatter : fmt::formatter diff --git a/src/projectmodel.cpp b/src/projectmodel.cpp index ca3c26b..05512b4 100644 --- a/src/projectmodel.cpp +++ b/src/projectmodel.cpp @@ -1,4 +1,5 @@ #include "projectmodel.h" +#include "db/abstracttimesheetrepository.h" #include "exceptions.h" #include "project.h" #include @@ -41,11 +42,16 @@ constexpr auto colors = std::array{ } // namespace -ProjectModel::ProjectModel() +ProjectModel::ProjectModel() : ProjectModel(null_repository()) { } -ProjectModel::ProjectModel(std::vector> projects) : m_projects(std::move(projects)) +ProjectModel::ProjectModel(AbstractTimeSheetRepository& repository) : m_repository(repository) +{ +} + +ProjectModel::ProjectModel(AbstractTimeSheetRepository& repository, std::vector> projects) + : m_repository(repository), m_projects(std::move(projects)) { } @@ -64,6 +70,9 @@ Project& ProjectModel::add(std::unique_ptr project) spdlog::info("Project {} has not color. Assigning {}.", project->name(), project->color().name()); } auto& ref = *m_projects.emplace_back(std::move(project)); + // Persist before notifying, so observers never see state the database does not have. A project + // that already carries an id is one an undone removal is putting back; insert() reuses it. + m_repository.insert(ref); Q_EMIT projects_changed(); return ref; } @@ -71,6 +80,13 @@ Project& ProjectModel::add(std::unique_ptr project) std::unique_ptr ProjectModel::extract(const Project& project) { const auto it = std::ranges::find(m_projects, &project, &std::unique_ptr::get); + if (it == m_projects.end()) { + throw RuntimeError("Cannot extract a project that this model does not own."); + } + // Delete the row before touching the container. If this throws, the model is unchanged and the + // caller gets nothing -- whereas removing first would destroy the project during unwinding and + // leave the calling command holding a dangling reference. + m_repository.remove(project); auto extracted_project = std::move(*it); m_projects.erase(it); Q_EMIT projects_changed(); diff --git a/src/projectmodel.h b/src/projectmodel.h index d65b630..14657d3 100644 --- a/src/projectmodel.h +++ b/src/projectmodel.h @@ -4,13 +4,17 @@ #include #include +class AbstractTimeSheetRepository; class Project; class ProjectModel : public QObject { Q_OBJECT public: + /** @brief Creates a model that does not persist anything. */ explicit ProjectModel(); - explicit ProjectModel(std::vector> projects); + explicit ProjectModel(AbstractTimeSheetRepository& repository); + /** @brief Adopts already-stored projects without writing them back. */ + explicit ProjectModel(AbstractTimeSheetRepository& repository, std::vector> projects); ~ProjectModel() override; [[nodiscard]] std::vector projects() const; @@ -24,5 +28,6 @@ class ProjectModel : public QObject void projects_changed(); private: + AbstractTimeSheetRepository& m_repository; std::vector> m_projects; }; diff --git a/src/serialization.cpp b/src/serialization.cpp deleted file mode 100644 index af003b5..0000000 --- a/src/serialization.cpp +++ /dev/null @@ -1,127 +0,0 @@ -#include "serialization.h" -#include "exceptions.h" -#include "intervalmodel.h" -#include "plan.h" -#include "projectmodel.h" -#include "timesheet.h" -#include -#include - -namespace -{ - -[[noreturn]] void throw_as_deserialization_error(const std::exception& e) -{ - throw DeserializationError("Failed to load time sheet: {}", e.what()); -} - -constexpr auto interval_model_key = "intervals"; -constexpr auto project_model_key = "projects"; -constexpr auto plan_key = "plan"; -constexpr auto project_key = "project"; -constexpr auto begin_key = "begin"; -constexpr auto end_key = "end"; - -using ProjectIndexMap = std::map; - -[[nodiscard]] auto create_project_index_map(const std::vector& projects) -{ - ProjectIndexMap map; - for (const auto* const project : projects) { - map.try_emplace(project, static_cast(map.size())); - } - return map; -} - -[[nodiscard]] auto serialize(const IntervalModel& interval_model, const ProjectIndexMap& project_index_map) -{ - std::list vs; - for (const auto* const interval : interval_model.intervals()) { - nlohmann::json& j = vs.emplace_back(); - j[begin_key] = interval->begin(); - j[end_key] = interval->end(); - if (const auto project = interval->project(); project == nullptr) { - j[project_key] = nullptr; - } else { - try { - j[project_key] = project_index_map.at(project); - } catch (std::out_of_range&) { - throw DeserializationError("Failed to store project reference."); - } - } - } - return vs; -} - -[[nodiscard]] auto serialize(const ProjectModel& project_model) -{ - std::list vs; - for (const auto* const project : project_model.projects()) { - vs.emplace_back(project->to_json()); - } - return vs; -} - -[[nodiscard]] auto deserialize_interval_model(const nlohmann::json& data, const std::vector& projects) -{ - std::deque> intervals; - for (const auto& v : data) { - try { - const auto project_reference = v.at(project_key); - const auto* const project = project_reference.is_null() ? nullptr : projects.at(project_reference); - auto& interval = *intervals.emplace_back(std::make_unique(project)); - interval.swap_begin(v.at(begin_key)); - interval.swap_end(v.at(end_key)); - } catch (const std::out_of_range&) { - throw DeserializationError("Failed to restore project reference."); - } catch (const nlohmann::json::exception&) { - throw DeserializationError("Failed to restore project reference."); - } - } - return std::make_unique(std::move(intervals)); -} - -[[nodiscard]] auto deserialize_project_model(const nlohmann::json& data) -{ - std::vector> projects; - projects.reserve(data.size()); - for (const auto& v : data) { - try { - projects.emplace_back(std::make_unique(v)); - } catch (const InvalidEnumNameException& e) { - throw DeserializationError("{}", e.what()); - } - } - try { - return std::make_unique(std::move(projects)); - } catch (const std::runtime_error& e) { - throw DeserializationError("{}", e.what()); - } -} - -} // namespace - -nlohmann::json serialize(const TimeSheet& time_sheet) -{ - nlohmann::json j; - j[project_model_key] = serialize(time_sheet.project_model()); - j[interval_model_key] = - serialize(time_sheet.interval_model(), ::create_project_index_map(time_sheet.project_model().projects())); - j[plan_key] = time_sheet.plan().to_json(); - return j; -} - -std::unique_ptr deserialize(const nlohmann::json& json) -{ - try { - auto project_model = ::deserialize_project_model(json.at(project_model_key)); - const auto projects = project_model->projects(); - auto interval_model = ::deserialize_interval_model(json.at(interval_model_key), projects); - auto plan = std::make_unique(json.at(plan_key)); - return std::make_unique(std::move(project_model), std::move(interval_model), std::move(plan)); - } catch (const nlohmann::json::out_of_range& e) { - ::throw_as_deserialization_error(e); - } catch (const RuntimeError& e) { - ::throw_as_deserialization_error(e); - } -} diff --git a/src/serialization.h b/src/serialization.h deleted file mode 100644 index 637e1af..0000000 --- a/src/serialization.h +++ /dev/null @@ -1,7 +0,0 @@ -#pragma once -#include "json.h" - -class TimeSheet; - -[[nodiscard]] nlohmann::json serialize(const TimeSheet& time_sheet); -[[nodiscard]] std::unique_ptr deserialize(const nlohmann::json& json); \ No newline at end of file diff --git a/src/timesheet.cpp b/src/timesheet.cpp index 215d294..4f19d4d 100644 --- a/src/timesheet.cpp +++ b/src/timesheet.cpp @@ -1,14 +1,19 @@ #include "timesheet.h" +#include "db/abstracttimesheetrepository.h" #include "intervalmodel.h" #include "period.h" #include "plan.h" #include "projectmodel.h" -TimeSheet::TimeSheet() - : m_project_model(std::make_unique()) - , m_interval_model(std::make_unique()) - , m_plan(std::make_unique()) +TimeSheet::TimeSheet() : TimeSheet(null_repository()) +{ +} + +TimeSheet::TimeSheet(AbstractTimeSheetRepository& repository) + : m_project_model(std::make_unique(repository)) + , m_interval_model(std::make_unique(repository)) + , m_plan(std::make_unique(repository)) { } @@ -18,6 +23,10 @@ TimeSheet::TimeSheet(std::unique_ptr project_model, std::unique_pt { } +TimeSheet::~TimeSheet() = default; +TimeSheet::TimeSheet(TimeSheet&&) noexcept = default; +TimeSheet& TimeSheet::operator=(TimeSheet&&) noexcept = default; + IntervalModel& TimeSheet::interval_model() const noexcept { return *m_interval_model; diff --git a/src/timesheet.h b/src/timesheet.h index 27b00e4..5e6000e 100644 --- a/src/timesheet.h +++ b/src/timesheet.h @@ -2,6 +2,7 @@ #include +class AbstractTimeSheetRepository; class Plan; class Period; class QDate; @@ -11,9 +12,18 @@ class ProjectModel; class TimeSheet { public: + /** @brief Creates an empty timesheet that does not persist anything. */ explicit TimeSheet(); + /** @brief Creates an empty timesheet whose models write through to @p repository. */ + explicit TimeSheet(AbstractTimeSheetRepository& repository); explicit TimeSheet(std::unique_ptr project_model, std::unique_ptr interval_model, std::unique_ptr plan); + // Declared here and defined in the .cpp so the models can stay forward-declared. + ~TimeSheet(); + TimeSheet(const TimeSheet&) = delete; + TimeSheet& operator=(const TimeSheet&) = delete; + TimeSheet(TimeSheet&&) noexcept; + TimeSheet& operator=(TimeSheet&&) noexcept; [[nodiscard]] IntervalModel& interval_model() const noexcept; [[nodiscard]] ProjectModel& project_model() const noexcept; [[nodiscard]] Plan& plan() const noexcept; diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt index 6299898..2b4e7f4 100644 --- a/test/unit/CMakeLists.txt +++ b/test/unit/CMakeLists.txt @@ -8,5 +8,10 @@ macro(package_add_test SOURCE_FILE) endmacro() package_add_test(colortest.cpp) +package_add_test(databasetest.cpp) +package_add_test(enumnamestest.cpp) package_add_test(periodtest.cpp) package_add_test(plantest.cpp) +package_add_test(sqlvaluetest.cpp) +package_add_test(timesheetrepositorytest.cpp) +package_add_test(undostacktransactiontest.cpp) diff --git a/test/unit/databasetest.cpp b/test/unit/databasetest.cpp new file mode 100644 index 0000000..4e4e65e --- /dev/null +++ b/test/unit/databasetest.cpp @@ -0,0 +1,234 @@ +#include "db/database.h" + +#include "db/migrations.h" +#include "exceptions.h" +#include "qtfixture.h" + +#include +#include +#include +#include +#include +#include +#include + +namespace +{ + +class DatabaseTest : public QtFixture +{ +protected: + /** @brief Probes for a table portably: sqlite_master is SQLite-only, information_schema is not. */ + [[nodiscard]] static bool has_table(const Database& database, const QString& table) + { + try { + // Selecting a literal rather than a column keeps the probe independent of each table's shape. + database.execute(QStringLiteral("SELECT 1 FROM %1 WHERE 1 = 0").arg(table)); + return true; + } catch (const DatabaseError&) { + return false; + } + } + + [[nodiscard]] static int count_rows(const Database& database, const QString& table) + { + auto query = database.prepare(QStringLiteral("SELECT COUNT(*) FROM %1").arg(table)); + database.execute(query); + EXPECT_TRUE(query.next()); + return query.value(0).toInt(); + } +}; + +} // namespace + +TEST_F(DatabaseTest, MigrateFromScratch) +{ + auto database = Database::open_in_memory(); + EXPECT_EQ(0, database.schema_version()); + database.migrate(); + EXPECT_EQ(latest_schema_version(), database.schema_version()); +} + +TEST_F(DatabaseTest, TablesExistAfterMigration) +{ + auto database = Database::open_in_memory(); + database.migrate(); + for (const auto* const table : {"project", "interval", "plan_entry", "plan_setting", "schema_version"}) { + EXPECT_TRUE(has_table(database, QString::fromLatin1(table))) << "missing table: " << table; + } +} + +TEST_F(DatabaseTest, MigrationIsIdempotent) +{ + auto database = Database::open_in_memory(); + database.migrate(); + const auto applied = count_rows(database, QStringLiteral("schema_version")); + database.migrate(); + EXPECT_EQ(latest_schema_version(), database.schema_version()); + EXPECT_EQ(applied, count_rows(database, QStringLiteral("schema_version"))); +} + +TEST_F(DatabaseTest, StepwiseMigration) +{ + auto database = Database::open_in_memory(); + database.migrate_to(1); + EXPECT_EQ(1, database.schema_version()); + database.migrate_to(latest_schema_version()); + EXPECT_EQ(latest_schema_version(), database.schema_version()); +} + +TEST_F(DatabaseTest, RejectsNewerSchema) +{ + auto database = Database::open_in_memory(); + database.migrate(); + + auto query = database.prepare(QStringLiteral("INSERT INTO schema_version (version, applied_at) VALUES (?, ?)")); + query.addBindValue(latest_schema_version() + 1); + query.addBindValue(QStringLiteral("2026-01-01T00:00:00")); + database.execute(query); + + // An older build must refuse a newer schema rather than silently mangling it. + EXPECT_THROW(database.migrate(), DatabaseError); +} + +TEST_F(DatabaseTest, ForeignKeysAreEnforced) +{ + // Canary for `PRAGMA foreign_keys = ON` silently not taking effect: SQLite ignores foreign keys + // per connection unless it is switched on, and then this insert would wrongly succeed. + auto database = Database::open_in_memory(); + database.migrate(); + + auto query = database.prepare(QStringLiteral("INSERT INTO interval (id, project_id, begin_time) VALUES (?, ?, ?)")); + query.addBindValue(1); + query.addBindValue(1234); // no such project + query.addBindValue(QStringLiteral("2026-01-01T08:00:00")); + EXPECT_THROW(database.execute(query), DatabaseError); +} + +TEST_F(DatabaseTest, RollbackOnUncommittedTransaction) +{ + auto database = Database::open_in_memory(); + database.migrate(); + { + Database::Transaction transaction{database}; + auto query = database.prepare(QStringLiteral("INSERT INTO project (id, name, color) VALUES (?, ?, ?)")); + query.addBindValue(1); + query.addBindValue(QStringLiteral("abandoned")); + query.addBindValue(QStringLiteral("#ff000000")); + database.execute(query); + // no commit() + } + EXPECT_EQ(0, count_rows(database, QStringLiteral("project"))); +} + +TEST_F(DatabaseTest, NestedTransactionCommitsOnce) +{ + auto database = Database::open_in_memory(); + database.migrate(); + + const auto insert_project = [&database](const int id, const QString& name) { + auto query = database.prepare(QStringLiteral("INSERT INTO project (id, name, color) VALUES (?, ?, ?)")); + query.addBindValue(id); + query.addBindValue(name); + query.addBindValue(QStringLiteral("#ff000000")); + database.execute(query); + }; + + { + Database::Transaction outer{database}; + { + Database::Transaction inner{database}; + insert_project(1, QStringLiteral("inner")); + inner.commit(); + } + insert_project(2, QStringLiteral("outer")); + outer.commit(); + } + EXPECT_EQ(2, count_rows(database, QStringLiteral("project"))); +} + +TEST_F(DatabaseTest, AbandonedInnerScopePoisonsTheWholeNest) +{ + auto database = Database::open_in_memory(); + database.migrate(); + { + Database::Transaction outer{database}; + { + Database::Transaction inner{database}; + auto query = database.prepare(QStringLiteral("INSERT INTO project (id, name, color) VALUES (?, ?, ?)")); + query.addBindValue(1); + query.addBindValue(QStringLiteral("doomed")); + query.addBindValue(QStringLiteral("#ff000000")); + database.execute(query); + // inner is abandoned, so the outer scope must not be able to commit half a compound change. + } + EXPECT_THROW(outer.commit(), DatabaseError); + } + EXPECT_EQ(0, count_rows(database, QStringLiteral("project"))); +} + +TEST_F(DatabaseTest, OpeningACorruptFileFails) +{ + // Must be reported, never "repaired" by silently recreating the file -- that would be data loss. + const QTemporaryDir directory; + ASSERT_TRUE(directory.isValid()); + const auto path = directory.filePath(QStringLiteral("corrupt.db")); + QFile file{path}; + ASSERT_TRUE(file.open(QIODevice::WriteOnly)); + file.write("this is definitely not a database"); + file.close(); + + // The rejection happens at open: SQLite only reads the file header on first access. + EXPECT_THROW( + { + auto database = Database::open_file(std::filesystem::path{path.toStdString()}); + database.migrate(); + }, + DatabaseError); + + // The file must be left exactly as it was, never "repaired" by recreating it. + QFile unchanged{path}; + ASSERT_TRUE(unchanged.open(QIODevice::ReadOnly)); + EXPECT_EQ("this is definitely not a database", unchanged.readAll()); +} + +TEST_F(DatabaseTest, OpeningInAnUnwritableDirectoryFails) +{ + if (::geteuid() == 0) { + GTEST_SKIP() << "running as root, which ignores directory permissions"; + } + const QTemporaryDir directory; + ASSERT_TRUE(directory.isValid()); + const auto sub_directory = QDir{directory.path()}.filePath(QStringLiteral("nowrite")); + ASSERT_TRUE(QDir{}.mkpath(sub_directory)); + ASSERT_TRUE(QFile::setPermissions(sub_directory, QFileDevice::ReadOwner | QFileDevice::ExeOwner)); + + const auto path = std::filesystem::path{QDir{sub_directory}.filePath(QStringLiteral("x.db")).toStdString()}; + EXPECT_THROW( + { + auto database = Database::open_file(path); + database.migrate(); + }, + DatabaseError); + + QFile::setPermissions(sub_directory, QFileDevice::ReadOwner | QFileDevice::WriteOwner | QFileDevice::ExeOwner); +} + +TEST_F(DatabaseTest, MigrationSurvivesReopen) +{ + const QTemporaryDir directory; + ASSERT_TRUE(directory.isValid()); + const auto path = std::filesystem::path{directory.filePath(QStringLiteral("tire.db")).toStdString()}; + + { + auto database = Database::open_file(path); + database.migrate(); + EXPECT_EQ(latest_schema_version(), database.schema_version()); + } + { + auto database = Database::open_file(path); + EXPECT_EQ(latest_schema_version(), database.schema_version()); + database.migrate(); // must be a no-op + EXPECT_EQ(1, count_rows(database, QStringLiteral("schema_version"))); + } +} diff --git a/test/unit/enumnamestest.cpp b/test/unit/enumnamestest.cpp new file mode 100644 index 0000000..f4f04b2 --- /dev/null +++ b/test/unit/enumnamestest.cpp @@ -0,0 +1,49 @@ +#include "db/enumnames.h" + +#include "exceptions.h" +#include "fmt.h" + +#include +#include +#include + +TEST(EnumNamesTest, PeriodTypeRoundTrip) +{ + using enum Period::Type; + for (const auto type : {Year, Month, Week, Day, Custom}) { + EXPECT_EQ(type, period_type_from_db_name(db_name(type))); + } +} + +TEST(EnumNamesTest, PlanKindRoundTrip) +{ + using enum Plan::Kind; + for (const auto kind : {Normal, Sick, Holiday, HalfHoliday, Vacation, HalfVacation, HalfVacationHalfHoliday}) { + EXPECT_EQ(kind, plan_kind_from_db_name(db_name(kind))); + } +} + +TEST(EnumNamesTest, NamesArePinned) +{ + // These literals are the on-disk format. Changing one silently orphans existing rows, so pin + // them here: a rename must be a deliberate migration, not an accident. + EXPECT_EQ(QStringLiteral("CUSTOM"), db_name(Period::Type::Custom)); + EXPECT_EQ(QStringLiteral("YEAR"), db_name(Period::Type::Year)); + EXPECT_EQ(QStringLiteral("NORMAL"), db_name(Plan::Kind::Normal)); + EXPECT_EQ(QStringLiteral("HALF_VACATION_HALF_HOLIDAY"), db_name(Plan::Kind::HalfVacationHalfHoliday)); +} + +TEST(EnumNamesTest, PersistenceNamesAreNotDisplayNames) +{ + // The fmt formatters go through QObject::tr(), so they are locale-dependent and unusable as a + // storage format. This guards against anyone re-coupling the two. + EXPECT_NE(QString::fromStdString(fmt::format("{}", Plan::Kind::HalfHoliday)), db_name(Plan::Kind::HalfHoliday)); +} + +TEST(EnumNamesTest, UnknownNameThrows) +{ + EXPECT_THROW(period_type_from_db_name(QStringLiteral("nonsense")), DatabaseError); + EXPECT_THROW(plan_kind_from_db_name(QStringLiteral("nonsense")), DatabaseError); + // Case matters: the stored form is upper case. + EXPECT_THROW(period_type_from_db_name(QStringLiteral("year")), DatabaseError); +} diff --git a/test/unit/plantest.cpp b/test/unit/plantest.cpp index 758025d..aff5492 100644 --- a/test/unit/plantest.cpp +++ b/test/unit/plantest.cpp @@ -1,5 +1,10 @@ #include "plan.h" +#include "application.h" +#include "commands/commands.h" +#include "commands/undostack.h" +#include "exceptions.h" + #include TEST(PlanTest, KindsIn) @@ -53,6 +58,62 @@ std::ostream& operator<<(std::ostream& o, const Plan::Kind kind) return o << name; } +TEST(PlanTest, can_set_period) +{ + FullTimePlan plan; + const auto add = [&plan](const QDate& begin, const QDate& end) { + plan.add(std::make_unique(Period{begin, end}, Plan::Kind::Holiday, EntityId{})); + }; + add(QDate{2025, 2, 1}, QDate{2025, 2, 3}); + add(QDate{2025, 3, 1}, QDate{2025, 3, 4}); + + // Keeping a period unchanged must be allowed: the entry may of course overlap itself. + EXPECT_TRUE(plan.can_set_period(plan.entry(0), Period{QDate{2025, 2, 1}, QDate{2025, 2, 3}})); + // Moving into free space. + EXPECT_TRUE(plan.can_set_period(plan.entry(0), Period{QDate{2025, 1, 1}, QDate{2025, 1, 5}})); + // Growing up to, but not into, the next entry. + EXPECT_TRUE(plan.can_set_period(plan.entry(0), Period{QDate{2025, 2, 1}, QDate{2025, 2, 28}})); + // Overlapping the other entry. + EXPECT_FALSE(plan.can_set_period(plan.entry(0), Period{QDate{2025, 2, 1}, QDate{2025, 3, 2}})); + // Merely touching the other entry counts as overlapping, matching the sortedness rule. + EXPECT_FALSE(plan.can_set_period(plan.entry(0), Period{QDate{2025, 2, 1}, QDate{2025, 3, 1}})); + + // can_set_period must agree with what swap_period actually does. + EXPECT_THROW(plan.swap_period(plan.entry(0), Period{QDate{2025, 2, 1}, QDate{2025, 3, 1}}), RuntimeError); + EXPECT_NO_THROW(plan.swap_period(plan.entry(0), Period{QDate{2025, 2, 1}, QDate{2025, 2, 28}})); +} + +TEST(PlanTest, undo_period_change_that_reorders) +{ + // Changing a period re-sorts the entries, so a command must not identify its entry by row + // index: after the reorder that index refers to a different entry, and undo would rewrite the + // wrong one. + FullTimePlan plan; + const auto add = [&plan](const QDate& begin, const QDate& end) { + plan.add(std::make_unique(Period{begin, end}, Plan::Kind::Holiday, EntityId{})); + }; + add(QDate{2026, 1, 5}, QDate{2026, 1, 5}); + add(QDate{2026, 3, 5}, QDate{2026, 3, 5}); + + // Identify the entries by address: Plan owns them through unique_ptrs, so sorting moves the + // pointers around but never the Entry objects themselves. + const auto* const moved = &plan.entry(0); + const auto* const other = &plan.entry(1); + + // Move the first entry past the second, which reorders them. + Application::undo_stack().push( + make_modify_plan_period_command(plan, *moved, Period{QDate{2026, 6, 5}, QDate{2026, 6, 5}})); + ASSERT_EQ(other, &plan.entry(0)) << "the entries should have swapped places"; + ASSERT_EQ(moved, &plan.entry(1)); + + Application::undo_stack().undo(); + + ASSERT_EQ(2, plan.rowCount({})); + EXPECT_EQ(Period(QDate(2026, 1, 5), QDate(2026, 1, 5)), moved->period) << "undo must restore the moved entry"; + EXPECT_EQ(Period(QDate(2026, 3, 5), QDate(2026, 3, 5)), other->period) << "undo must not touch the other entry"; + EXPECT_EQ(moved, &plan.entry(0)) << "and the original order should be back"; +} + TEST(PlanTest, add_sorted) { std::vector> periods; diff --git a/test/unit/qtfixture.h b/test/unit/qtfixture.h new file mode 100644 index 0000000..c2e4ddd --- /dev/null +++ b/test/unit/qtfixture.h @@ -0,0 +1,28 @@ +#pragma once + +#include +#include + +/** + * @class QtFixture qtfixture.h "qtfixture.h" + * @brief Provides the QCoreApplication that the Qt SQL drivers need. + * + * The drivers are Qt plugins, and plugin lookup requires a QCoreApplication instance. Without + * one, QSqlDatabase::addDatabase fails with "driver not loaded" even though the driver is + * installed. + * + * The instance is created once per test binary and intentionally never destroyed: tearing a + * QCoreApplication down while gtest still holds static state is not worth the risk, and the + * process is about to exit anyway. + */ +class QtFixture : public ::testing::Test +{ +public: + static void SetUpTestSuite() + { + static auto argc = 1; + static std::array argv{const_cast("tire-test"), static_cast(nullptr)}; + static auto* const application = new QCoreApplication{argc, argv.data()}; + Q_UNUSED(application) + } +}; diff --git a/test/unit/sqlvaluetest.cpp b/test/unit/sqlvaluetest.cpp new file mode 100644 index 0000000..22cf5f9 --- /dev/null +++ b/test/unit/sqlvaluetest.cpp @@ -0,0 +1,72 @@ +#include "db/sqlvalue.h" + +#include "exceptions.h" + +#include +#include +#include +#include + +TEST(SqlValueTest, DateTimeRoundTrip) +{ + const auto date_time = QDateTime{QDate{2026, 2, 3}, QTime{14, 35, 7}}; + EXPECT_EQ(date_time, date_time_from_sql(to_sql(date_time))); +} + +TEST(SqlValueTest, InvalidDateTimeIsNull) +{ + // This round-trip is what expresses "the interval is still running". + EXPECT_TRUE(to_sql(QDateTime{}).isNull()); + EXPECT_FALSE(date_time_from_sql(QVariant{}).isValid()); +} + +TEST(SqlValueTest, DateTimeIsFixedWidth) +{ + // Fixed width is what makes a lexicographic BETWEEN on begin_time a correct chronological + // range scan. A single-digit month or hour would break ordering. + const auto early = to_sql(QDateTime{QDate{2026, 2, 3}, QTime{4, 5, 6}}).toString(); + const auto late = to_sql(QDateTime{QDate{2026, 11, 30}, QTime{14, 35, 7}}).toString(); + EXPECT_EQ(19, early.length()); + EXPECT_EQ(early.length(), late.length()); + EXPECT_LT(early, late); +} + +TEST(SqlValueTest, DateRoundTrip) +{ + const auto date = QDate{2026, 2, 3}; + EXPECT_EQ(date, date_from_sql(to_sql(date))); + EXPECT_TRUE(to_sql(QDate{}).isNull()); + EXPECT_FALSE(date_from_sql(QVariant{}).isValid()); +} + +TEST(SqlValueTest, ColorRoundTripKeepsAlpha) +{ + // The previous JSON format used QColor::name(), which drops alpha silently. + const auto color = QColor{12, 34, 56, 78}; + const auto restored = color_from_sql(to_sql(color)); + EXPECT_EQ(color, restored); + EXPECT_EQ(78, restored.alpha()); +} + +TEST(SqlValueTest, ColorWithoutAlphaStillParses) +{ + const auto restored = color_from_sql(QVariant{QStringLiteral("#0c2238")}); + EXPECT_EQ(QColor(12, 34, 56), restored); +} + +TEST(SqlValueTest, MinutesRoundTripIncludingNegative) +{ + using namespace std::chrono_literals; + EXPECT_EQ(0min, minutes_from_sql(to_sql(0min))); + EXPECT_EQ(485min, minutes_from_sql(to_sql(485min))); + // Plan::overtime_offset may be negative. + EXPECT_EQ(-1234min, minutes_from_sql(to_sql(-1234min))); +} + +TEST(SqlValueTest, MalformedValuesThrow) +{ + EXPECT_THROW(date_time_from_sql(QVariant{QStringLiteral("not a date")}), DatabaseError); + EXPECT_THROW(date_from_sql(QVariant{QStringLiteral("not a date")}), DatabaseError); + EXPECT_THROW(color_from_sql(QVariant{QStringLiteral("not a color")}), DatabaseError); + EXPECT_THROW(minutes_from_sql(QVariant{QStringLiteral("not a number")}), DatabaseError); +} diff --git a/test/unit/timesheetrepositorytest.cpp b/test/unit/timesheetrepositorytest.cpp new file mode 100644 index 0000000..7d71f4d --- /dev/null +++ b/test/unit/timesheetrepositorytest.cpp @@ -0,0 +1,290 @@ +#include "db/sqltimesheetrepository.h" + +#include "db/database.h" +#include "exceptions.h" +#include "interval.h" +#include "intervalmodel.h" +#include "plan.h" +#include "project.h" +#include "projectmodel.h" +#include "qtfixture.h" +#include "timesheet.h" + +#include +#include +#include +#include + +namespace +{ + +using std::chrono_literals::operator""min; + +class TimeSheetRepositoryTest : public QtFixture +{ +protected: + void SetUp() override + { + m_database = std::make_unique(Database::open_in_memory()); + m_database->migrate(); + m_repository = std::make_unique(*m_database); + } + + [[nodiscard]] Project& add_project(const TimeSheet& time_sheet, const QString& name, const QColor& color) const + { + return time_sheet.project_model().add(std::make_unique(name, color)); + } + + [[nodiscard]] static Interval& add_interval(const TimeSheet& time_sheet, const Project* project, + const QDateTime& begin, const QDateTime& end) + { + auto interval = std::make_unique(project); + interval->swap_begin(begin); + interval->swap_end(end); + time_sheet.interval_model().add(std::move(interval)); + return time_sheet.interval_model().remove_const( + *time_sheet.interval_model().interval(time_sheet.interval_model().intervals().size() - 1)); + } + + std::unique_ptr m_database; + std::unique_ptr m_repository; +}; + +} // namespace + +TEST_F(TimeSheetRepositoryTest, RoundTripEmpty) +{ + const auto time_sheet = m_repository->load(); + EXPECT_TRUE(time_sheet->project_model().projects().empty()); + EXPECT_TRUE(time_sheet->interval_model().intervals().empty()); + EXPECT_EQ(0, time_sheet->plan().rowCount({})); + EXPECT_EQ(0min, time_sheet->plan().overtime_offset()); + // A fresh database must get its plan settings row, or every launch would silently re-pin the + // plan start date to "today" and overtime would never accumulate. + EXPECT_TRUE(time_sheet->plan().start().isValid()); +} + +TEST_F(TimeSheetRepositoryTest, ExtractAndReAddKeepsId) +{ + // The load-bearing test for putting the id on the entity rather than in a repository-side map: + // a command holds an extracted object alive and puts the *same* object back on undo, so the + // row -- and every foreign key pointing at it -- must survive the round trip. + const auto time_sheet = m_repository->load(); + auto& project = add_project(*time_sheet, QStringLiteral("recurring"), QColor{1, 2, 3}); + const auto original_id = project.id(); + ASSERT_TRUE(original_id.is_valid()); + + add_interval(*time_sheet, &project, QDateTime{QDate{2026, 3, 1}, QTime{8, 0}}, + QDateTime{QDate{2026, 3, 1}, QTime{16, 0}}); + + // Removing the project requires removing the referencing interval first (ON DELETE RESTRICT). + auto owned_interval = time_sheet->interval_model().extract(*time_sheet->interval_model().intervals().front()); + auto owned_project = time_sheet->project_model().extract(project); + EXPECT_EQ(original_id, owned_project->id()); + + // ... and this is the undo. + auto& re_added = time_sheet->project_model().add(std::move(owned_project)); + time_sheet->interval_model().add(std::move(owned_interval)); + EXPECT_EQ(original_id, re_added.id()); + + const auto reloaded = m_repository->load(); + ASSERT_EQ(1U, reloaded->project_model().projects().size()); + EXPECT_EQ(original_id, reloaded->project_model().projects().front()->id()); + ASSERT_EQ(1U, reloaded->interval_model().intervals().size()); + EXPECT_EQ(reloaded->project_model().projects().front(), reloaded->interval_model().intervals().front()->project()); +} + +TEST_F(TimeSheetRepositoryTest, RoundTripFull) +{ + const auto time_sheet = m_repository->load(); + auto& alpha = add_project(*time_sheet, QStringLiteral("alpha"), QColor{10, 20, 30, 128}); + auto& beta = add_project(*time_sheet, QStringLiteral("beta"), QColor{200, 100, 50}); + + add_interval(*time_sheet, &alpha, QDateTime{QDate{2026, 3, 1}, QTime{8, 0}}, + QDateTime{QDate{2026, 3, 1}, QTime{16, 30}}); + add_interval(*time_sheet, &beta, QDateTime{QDate{2026, 3, 2}, QTime{9, 15}}, + QDateTime{QDate{2026, 3, 2}, QTime{17, 45}}); + add_interval(*time_sheet, nullptr, QDateTime{QDate{2026, 3, 3}, QTime{7, 0}}, + QDateTime{QDate{2026, 3, 3}, QTime{8, 0}}); + add_interval(*time_sheet, &alpha, QDateTime{QDate{2026, 3, 4}, QTime{10, 0}}, {}); + + const auto reloaded = m_repository->load(); + + ASSERT_EQ(2U, reloaded->project_model().projects().size()); + EXPECT_EQ(QStringLiteral("alpha"), reloaded->project_model().projects().at(0)->name()); + EXPECT_EQ(QColor(10, 20, 30, 128), reloaded->project_model().projects().at(0)->color()); + EXPECT_EQ(128, reloaded->project_model().projects().at(0)->color().alpha()); + + const auto intervals = reloaded->interval_model().intervals(); + ASSERT_EQ(4U, intervals.size()); + EXPECT_EQ(QDateTime(QDate(2026, 3, 1), QTime(8, 0)), intervals.at(0)->begin()); + EXPECT_EQ(QDateTime(QDate(2026, 3, 1), QTime(16, 30)), intervals.at(0)->end()); +} + +TEST_F(TimeSheetRepositoryTest, ProjectPointersAreRebuilt) +{ + const auto time_sheet = m_repository->load(); + auto& alpha = add_project(*time_sheet, QStringLiteral("alpha"), QColor{10, 20, 30}); + auto& beta = add_project(*time_sheet, QStringLiteral("beta"), QColor{40, 50, 60}); + add_interval(*time_sheet, &beta, QDateTime{QDate{2026, 3, 1}, QTime{8, 0}}, + QDateTime{QDate{2026, 3, 1}, QTime{9, 0}}); + add_interval(*time_sheet, nullptr, QDateTime{QDate{2026, 3, 2}, QTime{8, 0}}, + QDateTime{QDate{2026, 3, 2}, QTime{9, 0}}); + Q_UNUSED(alpha) + + const auto reloaded = m_repository->load(); + const auto* const reloaded_beta = reloaded->project_model().projects().at(1); + ASSERT_EQ(QStringLiteral("beta"), reloaded_beta->name()); + // Identity, not just equality: the interval must point at the very object the model owns. + EXPECT_EQ(reloaded_beta, reloaded->interval_model().intervals().at(0)->project()); + EXPECT_EQ(nullptr, reloaded->interval_model().intervals().at(1)->project()); +} + +TEST_F(TimeSheetRepositoryTest, OngoingIntervalStaysOngoing) +{ + const auto time_sheet = m_repository->load(); + auto& project = add_project(*time_sheet, QStringLiteral("running"), QColor{1, 2, 3}); + add_interval(*time_sheet, &project, QDateTime{QDate{2026, 3, 4}, QTime{10, 0}}, {}); + + const auto reloaded = m_repository->load(); + ASSERT_EQ(1U, reloaded->interval_model().intervals().size()); + EXPECT_FALSE(reloaded->interval_model().intervals().front()->end().isValid()); + EXPECT_EQ(1U, reloaded->interval_model().open_intervals().size()); +} + +TEST_F(TimeSheetRepositoryTest, PeriodTypeSurvivesRoundTrip) +{ + const auto time_sheet = m_repository->load(); + time_sheet->plan().add( + std::make_unique(Period{QDate{2026, 5, 4}, Period::Type::Month}, Plan::Kind::Vacation, EntityId{})); + time_sheet->plan().add(std::make_unique(Period{QDate{2026, 8, 3}, QDate{2026, 8, 7}}, + Plan::Kind::HalfVacationHalfHoliday, EntityId{})); + + const auto reloaded = m_repository->load(); + ASSERT_EQ(2, reloaded->plan().rowCount({})); + + const auto& monthly = reloaded->plan().entry(0); + EXPECT_EQ(Period::Type::Month, monthly.period.type()); + EXPECT_EQ(QDate(2026, 5, 1), monthly.period.begin()); + EXPECT_EQ(QDate(2026, 5, 31), monthly.period.end()); + EXPECT_EQ(Plan::Kind::Vacation, monthly.kind); + + const auto& custom = reloaded->plan().entry(1); + EXPECT_EQ(Period::Type::Custom, custom.period.type()); + EXPECT_EQ(QDate(2026, 8, 3), custom.period.begin()); + EXPECT_EQ(QDate(2026, 8, 7), custom.period.end()); + EXPECT_EQ(Plan::Kind::HalfVacationHalfHoliday, custom.kind); +} + +TEST_F(TimeSheetRepositoryTest, EveryPlanKindSurvivesRoundTrip) +{ + const auto time_sheet = m_repository->load(); + using enum Plan::Kind; + constexpr auto kinds = + std::array{Normal, Sick, Holiday, HalfHoliday, Vacation, HalfVacation, HalfVacationHalfHoliday}; + for (auto i = 0U; i < kinds.size(); ++i) { + const auto day = QDate{2026, 6, static_cast(i) + 1}; + time_sheet->plan().add(std::make_unique(Period{day, day}, kinds.at(i), EntityId{})); + } + + const auto reloaded = m_repository->load(); + ASSERT_EQ(static_cast(kinds.size()), reloaded->plan().rowCount({})); + for (auto i = 0U; i < kinds.size(); ++i) { + EXPECT_EQ(kinds.at(i), reloaded->plan().entry(static_cast(i)).kind); + } +} + +TEST_F(TimeSheetRepositoryTest, PlanSettingsSurviveRoundTrip) +{ + const auto time_sheet = m_repository->load(); + time_sheet->plan().swap_start(QDate{2024, 1, 15}); + time_sheet->plan().swap_overtime_offset(-90min); + + const auto reloaded = m_repository->load(); + EXPECT_EQ(QDate(2024, 1, 15), reloaded->plan().start()); + EXPECT_EQ(-90min, reloaded->plan().overtime_offset()); +} + +TEST_F(TimeSheetRepositoryTest, PlanEntryOverlapIsRejectedOnLoad) +{ + auto insert_entry = [this](const int id, const QString& begin, const QString& end) { + auto query = m_database->prepare(QStringLiteral( + "INSERT INTO plan_entry (id, period_begin, period_end, period_type, kind) VALUES (?, ?, ?, 'CUSTOM', 'SICK')")); + query.addBindValue(id); + query.addBindValue(begin); + query.addBindValue(end); + m_database->execute(query); + }; + insert_entry(1, QStringLiteral("2026-01-01"), QStringLiteral("2026-01-10")); + insert_entry(2, QStringLiteral("2026-01-05"), QStringLiteral("2026-01-15")); + + EXPECT_THROW(m_repository->load(), DatabaseError); +} + +TEST_F(TimeSheetRepositoryTest, ModelsWorkWithoutDatabase) +{ + // The null-repository escape hatch: models must stay usable with no database at all, which is + // what keeps plantest and the benchmarks free of SQL. + const TimeSheet time_sheet; + EXPECT_NO_THROW({ + auto& project = time_sheet.project_model().add(std::make_unique(QStringLiteral("p"), QColor{1, 2, 3})); + auto interval = std::make_unique(&project); + interval->swap_begin(QDateTime{QDate{2026, 3, 1}, QTime{8, 0}}); + time_sheet.interval_model().add(std::move(interval)); + time_sheet.plan().add( + std::make_unique(Period{QDate{2026, 3, 5}, QDate{2026, 3, 6}}, Plan::Kind::Sick, EntityId{})); + time_sheet.plan().swap_overtime_offset(15min); + }); + EXPECT_EQ(1U, time_sheet.project_model().projects().size()); + EXPECT_EQ(1U, time_sheet.interval_model().intervals().size()); +} + +TEST_F(TimeSheetRepositoryTest, ReopenPersistsToDisk) +{ + // The only test that proves the durability story end to end: write, drop the connection + // entirely, reopen the file, and expect everything back. + const QTemporaryDir directory; + ASSERT_TRUE(directory.isValid()); + const auto path = std::filesystem::path{directory.filePath(QStringLiteral("tire.db")).toStdString()}; + + { + auto database = Database::open_file(path); + database.migrate(); + SqlTimeSheetRepository repository{database}; + const auto time_sheet = repository.load(); + auto& project = + time_sheet->project_model().add(std::make_unique(QStringLiteral("persisted"), QColor{9, 8, 7})); + auto interval = std::make_unique(&project); + interval->swap_begin(QDateTime{QDate{2026, 4, 1}, QTime{9, 0}}); + interval->swap_end(QDateTime{QDate{2026, 4, 1}, QTime{17, 0}}); + time_sheet->interval_model().add(std::move(interval)); + time_sheet->plan().swap_overtime_offset(42min); + } + + { + auto database = Database::open_file(path); + database.migrate(); + SqlTimeSheetRepository repository{database}; + const auto time_sheet = repository.load(); + ASSERT_EQ(1U, time_sheet->project_model().projects().size()); + EXPECT_EQ(QStringLiteral("persisted"), time_sheet->project_model().projects().front()->name()); + ASSERT_EQ(1U, time_sheet->interval_model().intervals().size()); + EXPECT_EQ(QDateTime(QDate(2026, 4, 1), QTime(17, 0)), time_sheet->interval_model().intervals().front()->end()); + EXPECT_EQ(42min, time_sheet->plan().overtime_offset()); + } +} + +TEST_F(TimeSheetRepositoryTest, NewIdsDoNotCollideAfterReload) +{ + { + const auto time_sheet = m_repository->load(); + add_project(*time_sheet, QStringLiteral("first"), QColor{1, 2, 3}); + add_project(*time_sheet, QStringLiteral("second"), QColor{4, 5, 6}); + } + // A repository constructed against an already-populated database must continue the id sequence + // rather than restart it, or the next insert collides with an existing primary key. + SqlTimeSheetRepository fresh_repository{*m_database}; + const auto time_sheet = fresh_repository.load(); + EXPECT_NO_THROW(time_sheet->project_model().add(std::make_unique(QStringLiteral("third"), QColor{7, 8, 9}))); + EXPECT_EQ(3U, fresh_repository.load()->project_model().projects().size()); +} diff --git a/test/unit/undostacktransactiontest.cpp b/test/unit/undostacktransactiontest.cpp new file mode 100644 index 0000000..b456368 --- /dev/null +++ b/test/unit/undostacktransactiontest.cpp @@ -0,0 +1,202 @@ +#include "application.h" + +#include "commands/addremovecommand.h" +#include "commands/commands.h" +#include "commands/undostack.h" +#include "db/sqltimesheetrepository.h" +#include "interval.h" +#include "intervalmodel.h" +#include "plan.h" +#include "project.h" +#include "projectmodel.h" +#include "timesheet.h" + +#include +#include + +/** + * Integration test for the path a real edit takes: + * UndoStack -> transaction -> model mutator -> repository -> database. + * + * The repository tests exercise the models directly; this one goes through the undo stack, which + * is where the transaction boundaries live and where undo and redo must also write through. + */ +namespace +{ + +class UndoStackTransactionTest : public ::testing::Test +{ +protected: + static void SetUpTestSuite() + { + // No display in CI, and none needed. + qputenv("QT_QPA_PLATFORM", "offscreen"); + static QTemporaryDir directory; + ASSERT_TRUE(directory.isValid()); + static const auto db_argument = + QStringLiteral("--database=%1").arg(directory.filePath(QStringLiteral("tire.db"))).toStdString(); + static auto arg0 = std::string{"tire-test"}; + static std::array argv{arg0.data(), const_cast(db_argument.c_str()), static_cast(nullptr)}; + static auto argc = 2; + // Leaked deliberately: a QApplication torn down while gtest still holds static state is not + // worth the risk, and the process exits right after. + static auto* const application = new Application{argc, argv.data()}; + const auto result = application->open_database(); + ASSERT_TRUE(result.ok) << result.message.toStdString(); + } + + void SetUp() override + { + m_time_sheet = Application::sql_repository().load(); + Application::undo_stack().impl().clear(); + } + + void TearDown() override + { + // Leave the database empty for the next test. + for (auto* const interval : m_time_sheet->interval_model().intervals()) { + m_time_sheet->interval_model().extract(*interval); + } + for (auto* const project : m_time_sheet->project_model().projects()) { + m_time_sheet->project_model().extract(*project); + } + m_time_sheet.reset(); + } + + [[nodiscard]] static std::unique_ptr make_interval(const Project* project, const QDateTime& begin) + { + auto interval = std::make_unique(project); + interval->swap_begin(begin); + return interval; + } + + /** @brief Interval count as the database sees it, not as the model does. */ + [[nodiscard]] static std::size_t stored_interval_count() + { + return Application::sql_repository().load()->interval_model().intervals().size(); + } + + std::unique_ptr m_time_sheet; +}; + +} // namespace + +TEST_F(UndoStackTransactionTest, PushWritesThrough) +{ + auto& model = m_time_sheet->interval_model(); + Application::undo_stack().push( + make(model, make_interval(nullptr, QDateTime{QDate{2026, 5, 1}, QTime{8, 0}}))); + EXPECT_EQ(1U, stored_interval_count()); +} + +TEST_F(UndoStackTransactionTest, UndoAndRedoWriteThrough) +{ + auto& model = m_time_sheet->interval_model(); + Application::undo_stack().push( + make(model, make_interval(nullptr, QDateTime{QDate{2026, 5, 1}, QTime{8, 0}}))); + ASSERT_EQ(1U, stored_interval_count()); + + Application::undo_stack().undo(); + EXPECT_EQ(0U, stored_interval_count()) << "undo must reach the database, not just memory"; + + Application::undo_stack().redo(); + EXPECT_EQ(1U, stored_interval_count()) << "redo must reach the database, not just memory"; +} + +TEST_F(UndoStackTransactionTest, ModifyingAnOwnedIntervalWritesThrough) +{ + auto& model = m_time_sheet->interval_model(); + Application::undo_stack().push( + make(model, make_interval(nullptr, QDateTime{QDate{2026, 5, 1}, QTime{8, 0}}))); + const auto end = QDateTime{QDate{2026, 5, 1}, QTime{17, 0}}; + Application::undo_stack().push( + make_modify_interval_command(model, *model.intervals().front(), end, &Interval::swap_end)); + + EXPECT_EQ(end, Application::sql_repository().load()->interval_model().intervals().front()->end()); + + Application::undo_stack().undo(); + EXPECT_FALSE(Application::sql_repository().load()->interval_model().intervals().front()->end().isValid()) + << "undoing a swap_end must clear the stored end too"; +} + +TEST_F(UndoStackTransactionTest, RetimingAnIntervalLaterWritesThrough) +{ + // Reproduces the ordinary "move this interval to a later time" edit. PeriodDetailView pushes + // swap_begin and swap_end as two separate commands, so the row passes through an intermediate + // state whose begin is after its end. The store must tolerate that, because the in-memory model + // does. + auto& model = m_time_sheet->interval_model(); + Application::undo_stack().push( + make(model, make_interval(nullptr, QDateTime{QDate{2026, 5, 1}, QTime{8, 0}}))); + Application::undo_stack().push(make_modify_interval_command( + model, *model.intervals().front(), QDateTime{QDate{2026, 5, 1}, QTime{9, 0}}, &Interval::swap_end)); + ASSERT_EQ(1U, stored_interval_count()); + + const auto new_begin = QDateTime{QDate{2026, 5, 1}, QTime{10, 0}}; + const auto new_end = QDateTime{QDate{2026, 5, 1}, QTime{11, 0}}; + { + const auto macro = Application::undo_stack().start_macro(QStringLiteral("Change interval")); + Application::undo_stack().push( + make_modify_interval_command(model, *model.intervals().front(), new_begin, &Interval::swap_begin)); + Application::undo_stack().push( + make_modify_interval_command(model, *model.intervals().front(), new_end, &Interval::swap_end)); + } + + const auto reloaded = Application::sql_repository().load(); + const auto stored = reloaded->interval_model().intervals(); + ASSERT_EQ(1U, stored.size()); + EXPECT_EQ(new_begin.toString(Qt::ISODate), stored.front()->begin().toString(Qt::ISODate)); + EXPECT_EQ(new_end.toString(Qt::ISODate), stored.front()->end().toString(Qt::ISODate)); + EXPECT_EQ(new_begin.toString(Qt::ISODate), model.intervals().front()->begin().toString(Qt::ISODate)) + << "in-memory value"; +} + +TEST_F(UndoStackTransactionTest, MacroIsOneUndoStepAndLandsCompletely) +{ + auto& model = m_time_sheet->interval_model(); + { + const auto macro = Application::undo_stack().start_macro(QStringLiteral("add three")); + for (auto day = 1; day <= 3; ++day) { + Application::undo_stack().push( + make(model, make_interval(nullptr, QDateTime{QDate{2026, 5, day}, QTime{8, 0}}))); + } + } + EXPECT_EQ(3U, stored_interval_count()); + + // One macro is one undo step, and it must roll the whole group back in the database as well. + Application::undo_stack().undo(); + EXPECT_EQ(0U, stored_interval_count()); + + Application::undo_stack().redo(); + EXPECT_EQ(3U, stored_interval_count()); +} + +TEST_F(UndoStackTransactionTest, PlanEditsAreUndoableAndPersisted) +{ + auto& plan = m_time_sheet->plan(); + plan.add(std::make_unique(Period{QDate{2026, 7, 1}, QDate{2026, 7, 3}}, Plan::Kind::Normal, EntityId{})); + ASSERT_EQ(1, Application::sql_repository().load()->plan().rowCount({})); + + Application::undo_stack().push(make_modify_plan_kind_command(plan, plan.entry(0), Plan::Kind::Sick)); + EXPECT_EQ(Plan::Kind::Sick, Application::sql_repository().load()->plan().entry(0).kind); + + // Plan edits used to bypass the undo stack entirely; they must not any more. + Application::undo_stack().undo(); + EXPECT_EQ(Plan::Kind::Normal, Application::sql_repository().load()->plan().entry(0).kind); + + for (auto* const entry_owner = &plan.entry(0); entry_owner != nullptr;) { + plan.extract(*entry_owner); + break; + } +} + +TEST_F(UndoStackTransactionTest, PlanSettingsArePersistedAndUndoable) +{ + auto& plan = m_time_sheet->plan(); + const auto original = plan.start(); + Application::undo_stack().push(make_modify_plan_start_command(plan, QDate{2023, 2, 1})); + EXPECT_EQ(QDate(2023, 2, 1), Application::sql_repository().load()->plan().start()); + + Application::undo_stack().undo(); + EXPECT_EQ(original, Application::sql_repository().load()->plan().start()); +} From 1e2dbdaeb308bd1f649b06ec8e9128e1854d78c7 Mon Sep 17 00:00:00 2001 From: pascal Date: Fri, 28 Aug 2026 17:59:43 +0200 Subject: [PATCH 02/12] allow to create the first project --- src/projectmodel.cpp | 6 + src/projectmodel.h | 8 ++ src/views/perioddetailview.cpp | 25 +++- test/unit/CMakeLists.txt | 1 + test/unit/projecteditortest.cpp | 187 ++++++++++++++++++++++++++ test/unit/timesheetrepositorytest.cpp | 23 ++++ 6 files changed, 244 insertions(+), 6 deletions(-) create mode 100644 test/unit/projecteditortest.cpp diff --git a/src/projectmodel.cpp b/src/projectmodel.cpp index 05512b4..65b877e 100644 --- a/src/projectmodel.cpp +++ b/src/projectmodel.cpp @@ -98,6 +98,12 @@ const Project& ProjectModel::project(const std::size_t index) const return *m_projects.at(index); } +const Project* ProjectModel::find(const QString& name) const +{ + const auto it = std::ranges::find(m_projects, name, [](const auto& project) { return project->name(); }); + return it == m_projects.end() ? nullptr : it->get(); +} + std::size_t ProjectModel::index_of(const Project& project) const { if (const auto it = std::ranges::find(m_projects, &project, &std::unique_ptr::get); it != m_projects.end()) { diff --git a/src/projectmodel.h b/src/projectmodel.h index 14657d3..41e7242 100644 --- a/src/projectmodel.h +++ b/src/projectmodel.h @@ -21,6 +21,14 @@ class ProjectModel : public QObject Project& add(std::unique_ptr project); std::unique_ptr extract(const Project& project); [[nodiscard]] const Project& project(std::size_t index) const; + + /** + * @brief The project called @p name, or nullptr if there is none. + * Name lookup is what the project editor needs: an editable combo box has no usable index for + * text the user just typed, and no index at all to distinguish "no project" from "not created + * yet". + */ + [[nodiscard]] const Project* find(const QString& name) const; [[nodiscard]] std::size_t index_of(const Project& project) const; [[nodiscard]] QColor generate_color() const; diff --git a/src/views/perioddetailview.cpp b/src/views/perioddetailview.cpp index 179584e..7ee4d2a 100644 --- a/src/views/perioddetailview.cpp +++ b/src/views/perioddetailview.cpp @@ -90,7 +90,7 @@ class ProjectItemDelegate final : public QStyledItemDelegate for (const auto& project : m_time_sheet->project_model().projects()) { editor->addItem(project->name()); } - editor->addItem(tr("No Project")); + editor->addItem(no_project_label()); return editor.release(); } @@ -106,19 +106,26 @@ class ProjectItemDelegate final : public QStyledItemDelegate void setModelData(QWidget* const editor, QAbstractItemModel* const model, const QModelIndex& index) const override { const auto& combo_box = dynamic_cast(*editor); + const auto current_text = combo_box.currentText(); + const auto* const known = m_time_sheet->project_model().find(current_text); + const Project* project = nullptr; - if (combo_box.currentIndex() >= m_time_sheet->project_model().projects().size()) { + if (known != nullptr) { + project = known; + } else if (current_text.isEmpty() || current_text == no_project_label()) { project = nullptr; - } else if (const auto current_text = combo_box.currentText(); - current_text == combo_box.itemText(combo_box.currentIndex())) - { - project = &m_time_sheet->project_model().project(combo_box.currentIndex()); } else if (QMessageBox::question(editor, QApplication::applicationDisplayName(), tr("There is no project '%1'. Do you want to create it?").arg(current_text), QMessageBox::Yes | QMessageBox::No) == QMessageBox::Yes) { + // Creating the project and assigning it are one user action, so they share a macro: one + // undo step, and one transaction. + const auto macro = Application::undo_stack().start_macro(tr("Assign new project")); project = &create_project(current_text); + Application::undo_stack().push(make_modify_interval_command(m_time_sheet->interval_model(), interval(index), + project, &Interval::swap_project)); + return; } else { return; } @@ -142,6 +149,12 @@ class ProjectItemDelegate final : public QStyledItemDelegate const TimeSheet* m_time_sheet = nullptr; PeriodDetailProxyModel& m_proxy_model; + /** @brief The sentinel entry standing for "this interval belongs to no project". */ + [[nodiscard]] static QString no_project_label() + { + return tr("No Project"); + } + [[nodiscard]] const Interval& interval(const QModelIndex& index) const { return *m_time_sheet->interval_model().interval(m_proxy_model.mapToSource(index).row()); diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt index 2b4e7f4..e990bfb 100644 --- a/test/unit/CMakeLists.txt +++ b/test/unit/CMakeLists.txt @@ -12,6 +12,7 @@ package_add_test(databasetest.cpp) package_add_test(enumnamestest.cpp) package_add_test(periodtest.cpp) package_add_test(plantest.cpp) +package_add_test(projecteditortest.cpp) package_add_test(sqlvaluetest.cpp) package_add_test(timesheetrepositorytest.cpp) package_add_test(undostacktransactiontest.cpp) diff --git a/test/unit/projecteditortest.cpp b/test/unit/projecteditortest.cpp new file mode 100644 index 0000000..a0ada48 --- /dev/null +++ b/test/unit/projecteditortest.cpp @@ -0,0 +1,187 @@ +#include "application.h" + +#include "commands/undostack.h" +#include "db/sqltimesheetrepository.h" +#include "interval.h" +#include "intervalmodel.h" +#include "period.h" +#include "project.h" +#include "projectmodel.h" +#include "timesheet.h" +#include "views/perioddetailview.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/** + * Drives the real project-column delegate, because the bug it guards against lived in how that + * delegate turned the combo box's state into a project -- not in anything the models could see. + */ +namespace +{ + +/** + * @brief Clicks a button on the next modal dialog that appears. + * setModelData asks for confirmation before creating a project, which would block the test. + */ +void answer_next_question(const QMessageBox::StandardButton button) +{ + auto* const timer = new QTimer; + timer->setInterval(10); + QObject::connect(timer, &QTimer::timeout, [timer, button] { + auto* const box = qobject_cast(QApplication::activeModalWidget()); + if (box == nullptr) { + return; + } + box->button(button)->click(); + timer->stop(); + timer->deleteLater(); + }); + timer->start(); +} + +class ProjectEditorTest : public ::testing::Test +{ +protected: + static void SetUpTestSuite() + { + qputenv("QT_QPA_PLATFORM", "offscreen"); + static QTemporaryDir directory; + ASSERT_TRUE(directory.isValid()); + static const auto db_argument = + QStringLiteral("--database=%1").arg(directory.filePath(QStringLiteral("tire.db"))).toStdString(); + static auto arg0 = std::string{"tire-test"}; + static std::array argv{arg0.data(), const_cast(db_argument.c_str()), static_cast(nullptr)}; + static auto argc = 2; + static auto* const application = new Application{argc, argv.data()}; + const auto result = application->open_database(); + ASSERT_TRUE(result.ok) << result.message.toStdString(); + } + + void SetUp() override + { + m_time_sheet = Application::sql_repository().load(); + Application::undo_stack().impl().clear(); + + // One interval on a known day, so the view has a row whose project cell can be edited. + auto interval = std::make_unique(nullptr); + interval->swap_begin(QDateTime{m_day, QTime{9, 0}}); + interval->swap_end(QDateTime{m_day, QTime{17, 0}}); + m_time_sheet->interval_model().add(std::move(interval)); + + m_view = std::make_unique(); + m_view->set_model(m_time_sheet.get()); + m_view->set_period(Period{m_day, Period::Type::Day}); + + m_table_view = m_view->findChild(); + ASSERT_NE(nullptr, m_table_view); + } + + void TearDown() override + { + m_view.reset(); + for (auto* const interval : m_time_sheet->interval_model().intervals()) { + m_time_sheet->interval_model().extract(*interval); + } + for (auto* const project : m_time_sheet->project_model().projects()) { + m_time_sheet->project_model().extract(*project); + } + m_time_sheet.reset(); + } + + /** @brief Types @p text into the project cell and commits it, exactly as the user does. */ + void type_project(const QString& text) const + { + const auto index = m_table_view->model()->index(0, project_column); + ASSERT_TRUE(index.isValid()); + auto* const delegate = m_table_view->itemDelegateForColumn(project_column); + ASSERT_NE(nullptr, delegate); + + auto* const editor = delegate->createEditor(m_table_view->viewport(), {}, index); + ASSERT_NE(nullptr, editor); + delegate->setEditorData(editor, index); + dynamic_cast(*editor).setCurrentText(text); + delegate->setModelData(editor, m_table_view->model(), index); + delete editor; + } + + static constexpr auto project_column = 0; + const QDate m_day{2026, 4, 7}; + std::unique_ptr m_time_sheet; + std::unique_ptr m_view; + QTableView* m_table_view = nullptr; +}; + +} // namespace + +TEST_F(ProjectEditorTest, CreatesTheVeryFirstProject) +{ + // The reported bug: with no projects yet, typing a name did nothing and the cell cleared. + ASSERT_TRUE(m_time_sheet->project_model().projects().empty()); + + answer_next_question(QMessageBox::Yes); + type_project(QStringLiteral("consulting")); + + ASSERT_EQ(1U, m_time_sheet->project_model().projects().size()); + EXPECT_EQ(QStringLiteral("consulting"), m_time_sheet->project_model().projects().front()->name()); + const auto* const assigned = m_time_sheet->interval_model().intervals().front()->project(); + ASSERT_NE(nullptr, assigned) << "the new project must also be assigned to the interval"; + EXPECT_EQ(QStringLiteral("consulting"), assigned->name()); + + // And it must have reached the database, not just the model. + const auto reloaded = Application::sql_repository().load(); + ASSERT_EQ(1U, reloaded->project_model().projects().size()); + EXPECT_EQ(QStringLiteral("consulting"), reloaded->project_model().projects().front()->name()); + ASSERT_NE(nullptr, reloaded->interval_model().intervals().front()->project()); +} + +TEST_F(ProjectEditorTest, CreatingASecondProjectStillWorks) +{ + answer_next_question(QMessageBox::Yes); + type_project(QStringLiteral("first")); + answer_next_question(QMessageBox::Yes); + type_project(QStringLiteral("second")); + + EXPECT_EQ(2U, m_time_sheet->project_model().projects().size()); + EXPECT_EQ(QStringLiteral("second"), m_time_sheet->interval_model().intervals().front()->project()->name()); +} + +TEST_F(ProjectEditorTest, PicksAnExistingProjectWithoutAsking) +{ + answer_next_question(QMessageBox::Yes); + type_project(QStringLiteral("existing")); + const auto* const created = m_time_sheet->project_model().projects().front(); + + // No dialog should appear this time; if one does, the test would hang, so nothing answers it. + type_project(QStringLiteral("existing")); + EXPECT_EQ(1U, m_time_sheet->project_model().projects().size()) << "must not create a duplicate"; + EXPECT_EQ(created, m_time_sheet->interval_model().intervals().front()->project()); +} + +TEST_F(ProjectEditorTest, DecliningCreatesNothing) +{ + answer_next_question(QMessageBox::No); + type_project(QStringLiteral("rejected")); + + EXPECT_TRUE(m_time_sheet->project_model().projects().empty()); + EXPECT_EQ(nullptr, m_time_sheet->interval_model().intervals().front()->project()); +} + +TEST_F(ProjectEditorTest, TheNoProjectSentinelClearsTheAssignment) +{ + answer_next_question(QMessageBox::Yes); + type_project(QStringLiteral("temporary")); + ASSERT_NE(nullptr, m_time_sheet->interval_model().intervals().front()->project()); + + // Must be understood as "no project", not as a project that needs creating. + type_project(QObject::tr("No Project")); + EXPECT_EQ(nullptr, m_time_sheet->interval_model().intervals().front()->project()); + EXPECT_EQ(1U, m_time_sheet->project_model().projects().size()) << "no project called 'No Project' may be created"; +} diff --git a/test/unit/timesheetrepositorytest.cpp b/test/unit/timesheetrepositorytest.cpp index 7d71f4d..4517cf6 100644 --- a/test/unit/timesheetrepositorytest.cpp +++ b/test/unit/timesheetrepositorytest.cpp @@ -94,6 +94,29 @@ TEST_F(TimeSheetRepositoryTest, ExtractAndReAddKeepsId) EXPECT_EQ(reloaded->project_model().projects().front(), reloaded->interval_model().intervals().front()->project()); } +TEST_F(TimeSheetRepositoryTest, CreatingTheFirstProjectWorks) +{ + // Regression: the project editor used to decide what the typed text meant from the combo box's + // row index. With no projects yet, index 0 is the "No Project" sentinel, so the very first + // project could never be created -- the typed name was silently discarded. Name lookup has to + // work on an empty model. + const auto time_sheet = m_repository->load(); + auto& projects = time_sheet->project_model(); + ASSERT_TRUE(projects.projects().empty()); + EXPECT_EQ(nullptr, projects.find(QStringLiteral("brand new"))); + + auto& created = add_project(*time_sheet, QStringLiteral("brand new"), QColor{1, 2, 3}); + EXPECT_EQ(&created, projects.find(QStringLiteral("brand new"))); + EXPECT_EQ(nullptr, projects.find(QStringLiteral("still missing"))); + // The sentinel label must never resolve to a real project. + EXPECT_EQ(nullptr, projects.find(QStringLiteral("No Project"))); + EXPECT_EQ(nullptr, projects.find(QString{})); + + const auto reloaded = m_repository->load(); + ASSERT_EQ(1U, reloaded->project_model().projects().size()); + EXPECT_EQ(QStringLiteral("brand new"), reloaded->project_model().projects().front()->name()); +} + TEST_F(TimeSheetRepositoryTest, RoundTripFull) { const auto time_sheet = m_repository->load(); From adef660c35830dfa93bc2d55d9ff69d0aa5bdb24 Mon Sep 17 00:00:00 2001 From: pascal Date: Fri, 28 Aug 2026 18:02:01 +0200 Subject: [PATCH 03/12] fix wayland tooltip issue --- src/ganttview.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ganttview.cpp b/src/ganttview.cpp index 39401d4..3cb6408 100644 --- a/src/ganttview.cpp +++ b/src/ganttview.cpp @@ -156,7 +156,7 @@ void GanttView::mouseMoveEvent(QMouseEvent* event) const auto kind_of_day = m_time_sheet->plan().find_kind(date_time.date()); const auto kind_of_day_text = kind_of_day == Plan::Kind::Normal ? "" : fmt::format(" [{}]", kind_of_day); QToolTip::showText(event->globalPosition().toPoint(), - date_time.toString("dddd, dd.MM. hh:mm") + QString::fromStdString(kind_of_day_text)); + date_time.toString("dddd, dd.MM. hh:mm") + QString::fromStdString(kind_of_day_text), this); } void GanttView::mousePressEvent(QMouseEvent* const event) From 5581cf69f283f48a39b84694672a6306b11d3392 Mon Sep 17 00:00:00 2001 From: pascal Date: Fri, 28 Aug 2026 18:18:53 +0200 Subject: [PATCH 04/12] improve splitter layout --- src/mainwindow.cpp | 35 +++++++++++++++++++++++++++++++++++ src/mainwindow.ui | 12 ++---------- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 240c6d1..9c98293 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -19,6 +19,20 @@ #include #include +namespace +{ + +// Pane order inside the main splitter, as laid out in mainwindow.ui. +constexpr auto plan_view_pane = 0; +constexpr auto period_detail_pane = 1; +constexpr auto tab_pane = 2; + +// How the space left over after the plan view is shared between the interval table and the tabs. +constexpr auto period_detail_share = 4; +constexpr auto tab_share = 3; + +} // namespace + MainWindow::MainWindow(std::unique_ptr time_sheet) : m_ui(std::make_unique()), m_view_action_group(this) { @@ -34,6 +48,27 @@ MainWindow::MainWindow(std::unique_ptr time_sheet) connect(&Application::undo_stack(), &UndoStack::write_failed, this, &MainWindow::on_write_failed); connect(&Application::undo_stack(), &UndoStack::write_succeeded, this, &MainWindow::on_write_succeeded); + // One splitter across all three panes, so every boundary is draggable and there is no nesting. + // + // The plan view is a fixed set of summary labels: it has a natural width and gains nothing from + // being wider, so it takes no share of spare space and the other two divide it between them. + // + // Stretch factors alone only govern how *surplus* is shared when the window is resized -- the + // initial split is proportional to the size hints, which left the plan view far wider than it + // needs. Asking for its size hint and oversized values for the rest pins it: the splitter + // clamps those down to the space actually left, in the ratio given. + m_ui->splitter->setStretchFactor(plan_view_pane, 0); + m_ui->splitter->setStretchFactor(period_detail_pane, period_detail_share); + m_ui->splitter->setStretchFactor(tab_pane, tab_share); + // The summary and the table are the primary views and must not vanish by accident; the tab pane + // may be dragged shut, as it could be before. + m_ui->splitter->setCollapsible(plan_view_pane, false); + m_ui->splitter->setCollapsible(period_detail_pane, false); + m_ui->splitter->setCollapsible(tab_pane, true); + static constexpr auto total_share = period_detail_share + tab_share; + m_ui->splitter->setSizes({m_ui->plan_view->sizeHint().width(), period_detail_share * QWIDGETSIZE_MAX / total_share, + tab_share * QWIDGETSIZE_MAX / total_share}); + m_ui->period_detail_view->setContextMenuPolicy(Qt::CustomContextMenu); connect(m_ui->period_detail_view, &PeriodDetailView::current_interval_changed, m_ui->ganttview, &GanttView::set_current_interval); diff --git a/src/mainwindow.ui b/src/mainwindow.ui index 3afad4a..018724b 100644 --- a/src/mainwindow.ui +++ b/src/mainwindow.ui @@ -35,16 +35,8 @@ Qt::Orientation::Horizontal - - - - - - - - - - + + 0 From 56df683ce54046d56a94f609ed2ff231b21dfb28 Mon Sep 17 00:00:00 2001 From: pascal Date: Fri, 28 Aug 2026 19:37:55 +0200 Subject: [PATCH 05/12] make TimeRangeEditor more intuitive --- src/timerangeeditor.cpp | 40 +++------------------------------- src/timerangeeditor.h | 4 +--- src/timerangeeditor.ui | 32 ++------------------------- src/views/perioddetailview.cpp | 11 +++++++--- src/views/perioddetailview.h | 2 +- 5 files changed, 15 insertions(+), 74 deletions(-) diff --git a/src/timerangeeditor.cpp b/src/timerangeeditor.cpp index aa48cf9..8e8e4bb 100644 --- a/src/timerangeeditor.cpp +++ b/src/timerangeeditor.cpp @@ -1,41 +1,14 @@ #include "timerangeeditor.h" #include "application.h" -#include "datetimeselector.h" -#include "intervalmodel.h" #include "ui_timerangeeditor.h" #include -TimeRangeEditor::TimeRangeEditor(const IntervalModel& interval_model, QWidget* const parent) - : QDialog(parent), m_ui(std::make_unique()) +TimeRangeEditor::TimeRangeEditor(QWidget* const parent) : QDialog(parent), m_ui(std::make_unique()) { m_ui->setupUi(this); connect(m_ui->cb_has_end, &QCheckBox::toggled, this, &TimeRangeEditor::update_enabledness); - connect(m_ui->pb_begin_to_last_end, &QPushButton::clicked, this, [&interval_model, this]() { - auto ends_view = - interval_model.intervals() | std::views::transform(&Interval::end) | std::views::filter(&QDateTime::isValid); - if (const std::vector ends(ends_view.begin(), ends_view.end()); ends.empty()) { - QMessageBox::critical(this, "Error", "No end intervals were found"); - } else { - const auto end = std::ranges::max_element(ends); - m_ui->de_begin->setDate(end->date()); - m_ui->te_begin->set_time(end->time()); - } - }); - connect(m_ui->pb_begin_to_now, &QPushButton::clicked, this, [this]() { - m_ui->te_begin->set_time(Application::current_date_time().time()); - m_ui->de_begin->setDate(Application::current_date_time().date()); - }); - connect(m_ui->pb_end_to_begin, &QPushButton::clicked, this, [this]() { - m_ui->te_end->set_time(m_ui->te_begin->time()); - m_ui->sp_end_offset->setValue(0); - }); - connect(m_ui->pb_end_to_now, &QPushButton::clicked, this, [this]() { - const auto offset = m_ui->de_begin->date().daysTo(Application::current_date_time().date()); - m_ui->sp_end_offset->setValue(offset); - m_ui->te_end->set_time(Application::current_date_time().time()); - }); update_enabledness(); } @@ -64,17 +37,10 @@ QDateTime TimeRangeEditor::end() const noexcept : QDateTime{}; } -void TimeRangeEditor::set_end(const QDateTime& end) -{ - set_range(begin(), end); -} - void TimeRangeEditor::update_enabledness() const { - for (auto* const w : - std::vector{m_ui->pb_end_to_begin, m_ui->pb_end_to_now, m_ui->te_end, m_ui->sp_end_offset}) - { - w->setEnabled(m_ui->cb_has_end->isChecked()); + for (auto* const widget : std::vector{m_ui->te_end, m_ui->sp_end_offset}) { + widget->setEnabled(m_ui->cb_has_end->isChecked()); } } diff --git a/src/timerangeeditor.h b/src/timerangeeditor.h index f3aaafe..6a5a4c9 100644 --- a/src/timerangeeditor.h +++ b/src/timerangeeditor.h @@ -3,7 +3,6 @@ #include #include -class IntervalModel; namespace Ui { class TimeRangeEditor; @@ -14,13 +13,12 @@ class TimeRangeEditor : public QDialog Q_OBJECT public: - explicit TimeRangeEditor(const IntervalModel& interval_model, QWidget* parent = nullptr); + explicit TimeRangeEditor(QWidget* parent); ~TimeRangeEditor() override; void set_range(const QDateTime& begin, const QDateTime& end); [[nodiscard]] QDateTime begin() const noexcept; [[nodiscard]] QDateTime end() const noexcept; - void set_end(const QDateTime& end); void accept() override; private: diff --git a/src/timerangeeditor.ui b/src/timerangeeditor.ui index b9213a2..7208369 100644 --- a/src/timerangeeditor.ui +++ b/src/timerangeeditor.ui @@ -14,13 +14,6 @@ Edit Time Range - - - - Now - - - @@ -34,13 +27,6 @@ - - - - to Begin - - - @@ -58,7 +44,7 @@ - + Qt::Orientation::Vertical @@ -81,24 +67,10 @@ - - - - to last End - - - - - - - Now - - - - + QDialogButtonBox::StandardButton::Cancel|QDialogButtonBox::StandardButton::Ok diff --git a/src/views/perioddetailview.cpp b/src/views/perioddetailview.cpp index 7ee4d2a..a4f47c9 100644 --- a/src/views/perioddetailview.cpp +++ b/src/views/perioddetailview.cpp @@ -272,11 +272,16 @@ void PeriodDetailView::show_table_context_menu(const QPoint& pos) menu.exec(pos); } -void PeriodDetailView::edit_date_time(const QModelIndex& index) const +void PeriodDetailView::edit_date_time(const QModelIndex& index) { - TimeRangeEditor e{time_sheet()->interval_model()}; + TimeRangeEditor e{this}; const auto& interval = *time_sheet()->interval_model().interval(index.row()); - e.set_range(interval.begin(), interval.end()); + // Clicking the end column of a still-running interval means the user wants to end it: hand the + // dialog an end of "now" so the field is filled in and editable, rather than a disabled one + // they must first hunt down the checkbox to unlock. + const auto ends_a_running_interval = !interval.end().isValid() && index.column() == IntervalModel::end_column; + const auto end = ends_a_running_interval ? Application::current_date_time() : interval.end(); + e.set_range(interval.begin(), end); if (e.exec() != QDialog::Accepted) { return; } diff --git a/src/views/perioddetailview.h b/src/views/perioddetailview.h index 5de5683..540538f 100644 --- a/src/views/perioddetailview.h +++ b/src/views/perioddetailview.h @@ -35,5 +35,5 @@ class PeriodDetailView final : public AbstractPeriodView void init_context_menu_actions(); void show_table_context_menu(const QPoint& pos); - void edit_date_time(const QModelIndex& index) const; + void edit_date_time(const QModelIndex& index); }; From 6612483dce0d5a64dd549d931f6eefe99cedd489 Mon Sep 17 00:00:00 2001 From: pascal Date: Fri, 28 Aug 2026 19:39:09 +0200 Subject: [PATCH 06/12] fix tab order --- src/timeedit.cpp | 10 ++++++++++ src/timeedit.h | 11 +++++++++++ src/timerangeeditor.cpp | 12 ++++++++++++ src/timerangeeditor.ui | 5 +++++ 4 files changed, 38 insertions(+) diff --git a/src/timeedit.cpp b/src/timeedit.cpp index a4a10ad..7ecc74b 100644 --- a/src/timeedit.cpp +++ b/src/timeedit.cpp @@ -51,3 +51,13 @@ void TimeEdit::handle_change() m_ui->sp_min->set_block_wrap_up(h == m_max.hour()); m_ui->sp_min->setRange(min_m, max_m); } + +QWidget* TimeEdit::first_focus_widget() const +{ + return m_ui->sp_h; +} + +QWidget* TimeEdit::last_focus_widget() const +{ + return m_ui->sp_min; +} diff --git a/src/timeedit.h b/src/timeedit.h index c0cc0d1..952e358 100644 --- a/src/timeedit.h +++ b/src/timeedit.h @@ -19,6 +19,17 @@ class TimeEdit : public QWidget void set_time(const QTime& time) noexcept; void set_time_range(const QTime& min, const QTime& max) noexcept; + /** + * @name Focus chain endpoints + * TimeEdit is a compound widget whose focusable children live in its own .ui file, so a + * containing dialog's cannot name them and they end up stranded at the end of the + * focus chain. These let the dialog weave the widget into its tab order with setTabOrder(). + * @{ + */ + [[nodiscard]] QWidget* first_focus_widget() const; + [[nodiscard]] QWidget* last_focus_widget() const; + /** @} */ + Q_SIGNALS: void time_changed(); diff --git a/src/timerangeeditor.cpp b/src/timerangeeditor.cpp index 8e8e4bb..387475e 100644 --- a/src/timerangeeditor.cpp +++ b/src/timerangeeditor.cpp @@ -8,6 +8,18 @@ TimeRangeEditor::TimeRangeEditor(QWidget* const parent) : QDialog(parent), m_ui(std::make_unique()) { m_ui->setupUi(this); + + // The .ui file's chain de_begin -> cb_has_end -> sp_end_offset. The two TimeEdits + // cannot be named there: they are compound widgets whose focusable children live in + // timeedit.ui, so they would otherwise be stranded at the end of the chain, after the buttons + // and in reverse order. Weave them in here so focus follows the visual grid. + setTabOrder(m_ui->de_begin, m_ui->te_begin->first_focus_widget()); + setTabOrder(m_ui->te_begin->first_focus_widget(), m_ui->te_begin->last_focus_widget()); + setTabOrder(m_ui->te_begin->last_focus_widget(), m_ui->cb_has_end); + setTabOrder(m_ui->sp_end_offset, m_ui->te_end->first_focus_widget()); + setTabOrder(m_ui->te_end->first_focus_widget(), m_ui->te_end->last_focus_widget()); + setTabOrder(m_ui->te_end->last_focus_widget(), m_ui->buttonBox); + connect(m_ui->cb_has_end, &QCheckBox::toggled, this, &TimeRangeEditor::update_enabledness); update_enabledness(); } diff --git a/src/timerangeeditor.ui b/src/timerangeeditor.ui index 7208369..38a90bb 100644 --- a/src/timerangeeditor.ui +++ b/src/timerangeeditor.ui @@ -87,6 +87,11 @@ 1 + + de_begin + cb_has_end + sp_end_offset + From 28cfa738ae491fea2b73c8480c845b5f1324b464 Mon Sep 17 00:00:00 2001 From: pascal Date: Fri, 28 Aug 2026 19:52:51 +0200 Subject: [PATCH 07/12] fix invisible text on new interval rows --- src/views/perioddetailview.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/views/perioddetailview.cpp b/src/views/perioddetailview.cpp index a4f47c9..b7d95dd 100644 --- a/src/views/perioddetailview.cpp +++ b/src/views/perioddetailview.cpp @@ -30,8 +30,15 @@ void init_style_selected(QStyleOptionViewItem* option, const QModelIndex& index) if (option->state & QStyle::State_Selected) { option->font.setBold(true); option->font.setUnderline(true); - option->palette.setBrush(QPalette::Highlight, index.data(Qt::BackgroundRole).value()); - option->palette.setBrush(QPalette::HighlightedText, index.data(Qt::ForegroundRole).value()); + // An interval without a project supplies neither role, and QVariant{}.value() is an + // opaque black one -- which would paint black text on a black selection. Keep the palette's + // own highlight colors in that case. The two are taken together because the foreground is + // chosen to contrast with the background; mixing one with the palette's other loses that. + const auto background = index.data(Qt::BackgroundRole); + if (const auto foreground = index.data(Qt::ForegroundRole); background.isValid() && foreground.isValid()) { + option->palette.setBrush(QPalette::Highlight, background.value()); + option->palette.setBrush(QPalette::HighlightedText, foreground.value()); + } } } From 79b401d35d45dd9c2fd0235c739025642c585703 Mon Sep 17 00:00:00 2001 From: pascal Date: Fri, 28 Aug 2026 20:05:30 +0200 Subject: [PATCH 08/12] focus begin/end time depending on the clicked column --- src/timerangeeditor.cpp | 11 +++++++++++ src/timerangeeditor.h | 2 ++ src/views/perioddetailview.cpp | 7 +++++++ 3 files changed, 20 insertions(+) diff --git a/src/timerangeeditor.cpp b/src/timerangeeditor.cpp index 387475e..c4f51e2 100644 --- a/src/timerangeeditor.cpp +++ b/src/timerangeeditor.cpp @@ -49,6 +49,17 @@ QDateTime TimeRangeEditor::end() const noexcept : QDateTime{}; } +void TimeRangeEditor::focus_begin_time() +{ + m_ui->te_begin->first_focus_widget()->setFocus(); +} + +void TimeRangeEditor::focus_end_time() +{ + // Only has an effect while the end is enabled, i.e. the interval is not marked as running. + m_ui->te_end->first_focus_widget()->setFocus(); +} + void TimeRangeEditor::update_enabledness() const { for (auto* const widget : std::vector{m_ui->te_end, m_ui->sp_end_offset}) { diff --git a/src/timerangeeditor.h b/src/timerangeeditor.h index 6a5a4c9..d7009af 100644 --- a/src/timerangeeditor.h +++ b/src/timerangeeditor.h @@ -19,6 +19,8 @@ class TimeRangeEditor : public QDialog void set_range(const QDateTime& begin, const QDateTime& end); [[nodiscard]] QDateTime begin() const noexcept; [[nodiscard]] QDateTime end() const noexcept; + void focus_begin_time(); + void focus_end_time(); void accept() override; private: diff --git a/src/views/perioddetailview.cpp b/src/views/perioddetailview.cpp index b7d95dd..8ffb468 100644 --- a/src/views/perioddetailview.cpp +++ b/src/views/perioddetailview.cpp @@ -289,6 +289,13 @@ void PeriodDetailView::edit_date_time(const QModelIndex& index) const auto ends_a_running_interval = !interval.end().isValid() && index.column() == IntervalModel::end_column; const auto end = ends_a_running_interval ? Application::current_date_time() : interval.end(); e.set_range(interval.begin(), end); + // Start on the field the user clicked. Must follow set_range, which decides whether the end is + // enabled at all. + if (index.column() == IntervalModel::end_column) { + e.focus_end_time(); + } else if (index.column() == IntervalModel::begin_column) { + e.focus_begin_time(); + } if (e.exec() != QDialog::Accepted) { return; } From 019d7c251f53e05a1c9171bda7a7f4f8a6ef3743 Mon Sep 17 00:00:00 2001 From: pascal Date: Fri, 28 Aug 2026 20:29:46 +0200 Subject: [PATCH 09/12] fix build for unit tests --- .github/workflows/cmake-multi-platform.yml | 6 ++++-- .gitmodules | 2 +- CMakeLists.txt | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/cmake-multi-platform.yml b/.github/workflows/cmake-multi-platform.yml index 03af1b5..2eec75c 100644 --- a/.github/workflows/cmake-multi-platform.yml +++ b/.github/workflows/cmake-multi-platform.yml @@ -42,6 +42,8 @@ jobs: steps: - uses: actions/checkout@v4 + with: + submodules: recursive - if: ${{ matrix.os == 'ubuntu-latest' }} run: sudo apt-get update; sudo apt-get install ninja-build @@ -108,8 +110,8 @@ jobs: - name: Build run: | - cmake -G "${{ matrix.generator }}" -S . -B build -DCMAKE_TOOLCHAIN_FILE="build/build/generators/conan_toolchain.cmake" -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} + cmake -G "${{ matrix.generator }}" -S . -B build -DCMAKE_TOOLCHAIN_FILE="build/build/generators/conan_toolchain.cmake" -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DBUILD_TESTING=ON cmake --build build --config ${{ matrix.build_type }} --parallel - name: Test - run: ctest --build-config ${{ matrix.build_type }} --test-dir build + run: ctest --build-config ${{ matrix.build_type }} --test-dir build --output-on-failure diff --git a/.gitmodules b/.gitmodules index 09d09ff..8cf8b5e 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,3 @@ [submodule "googletest"] path = googletest - url = git@github.com:google/googletest.git + url = https://github.com/google/googletest.git diff --git a/CMakeLists.txt b/CMakeLists.txt index 25e4c4d..4937f55 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,10 +11,11 @@ set(CMAKE_AUTOMOC ON) set(CMAKE_AUTOUIC ON) set(CMAKE_AUTORCC ON) +include(CTest) + add_subdirectory(src) if (BUILD_TESTING) add_subdirectory(googletest) add_subdirectory(benchmarks) - include(CTest) add_subdirectory(test/unit) endif () From b61be60fece3d47c968d96bf00f6cfaca0501a02 Mon Sep 17 00:00:00 2001 From: pascal Date: Fri, 28 Aug 2026 20:43:34 +0200 Subject: [PATCH 10/12] fix CI build --- .clang-format-ignore | 2 + .github/workflows/cmake-multi-platform.yml | 155 +++++++++++---------- src/views/perioddetailproxymodel.h | 2 +- 3 files changed, 85 insertions(+), 74 deletions(-) create mode 100644 .clang-format-ignore diff --git a/.clang-format-ignore b/.clang-format-ignore new file mode 100644 index 0000000..4b935b5 --- /dev/null +++ b/.clang-format-ignore @@ -0,0 +1,2 @@ +# Vendored from https://github.com/KDAB/KDSingleApplication — keep upstream formatting. +src/kdsingleapplication/* diff --git a/.github/workflows/cmake-multi-platform.yml b/.github/workflows/cmake-multi-platform.yml index 2eec75c..6fb2b13 100644 --- a/.github/workflows/cmake-multi-platform.yml +++ b/.github/workflows/cmake-multi-platform.yml @@ -7,111 +7,120 @@ on: branches: [ "main" ] pull_request: branches: [ "main" ] + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: + format: + runs-on: ubuntu-latest + timeout-minutes: 10 + + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: '3.12' + + - name: Install clang-format + run: python -m pip install --upgrade "clang-format==22.1.8" + + - name: Check formatting + run: git ls-files '*.cpp' '*.h' | xargs clang-format --dry-run --Werror + build: env: CONAN_HOME: "${{ github.workspace }}/.conan2" runs-on: ${{ matrix.os }} + timeout-minutes: 120 strategy: # Set fail-fast to false to ensure that feedback is delivered for all matrix combinations. Consider changing this to true when your workflow is stable. fail-fast: false matrix: - os: [ubuntu-latest, windows-latest] - compiler: [gcc, msvc] - build_type: [Release] - exclude: - - os: windows-latest - compiler: gcc - - os: ubuntu-latest - compiler: msvc include: - - compiler: gcc - generator: "Unix Makefiles" + - os: ubuntu-latest + build_type: Release cppstd: gnu20 - libcxx: libstdc++ - compiler_version: 11 - - compiler: msvc - generator: "Visual Studio 17 2022" + cmake_generator: Ninja + configure_preset: conan-release + build_preset: conan-release + build_dir: build/Release + - os: windows-latest + build_type: Release cppstd: 20 - compiler_runtime: dynamic - compiler_version: 194 + cmake_generator: Visual Studio 17 2022 + configure_preset: conan-default + build_preset: conan-release + build_dir: build steps: - uses: actions/checkout@v4 with: submodules: recursive - - if: ${{ matrix.os == 'ubuntu-latest' }} - run: sudo apt-get update; sudo apt-get install ninja-build + - name: Install Ninja + if: runner.os == 'Linux' + run: sudo apt-get update && sudo apt-get install -y ninja-build - - name: Install Conan - id: conan - uses: turtlebrowser/get-conan@main + - uses: actions/setup-python@v5 + with: + python-version: '3.12' - - name: Conan version - run: echo "${{ steps.conan.outputs.version }}" + - name: Install Conan + run: python -m pip install --upgrade "conan>=2,<3" - - name: Cache Conan Dependencies + # cache/restore + cache/save rather than actions/cache: conanfile.py pins version + # ranges, so an unchanged recipe can still resolve to new dependencies. A plain + # cache would keep hitting its key and never save the newly built packages. + - name: Restore Conan cache id: cache-conan - uses: actions/cache@v4 + uses: actions/cache/restore@v4 with: path: ${{ env.CONAN_HOME }} - key: conan-cache-${{ runner.os }}-${{ hashFiles('conanfile.py') }} - restore-keys: conan-cache-${{ runner.os }}- + key: conan-${{ runner.os }}-${{ matrix.build_type }}-${{ matrix.cppstd }}-${{ hashFiles('conanfile.py') }}-${{ github.run_id }} + restore-keys: | + conan-${{ runner.os }}-${{ matrix.build_type }}-${{ matrix.cppstd }}-${{ hashFiles('conanfile.py') }}- + conan-${{ runner.os }}-${{ matrix.build_type }}-${{ matrix.cppstd }}- - - uses: DamianReeves/write-file-action@master - if: ${{ matrix.os == 'ubuntu-latest' }} - with: - path: ${{ env.CONAN_HOME }}/profiles/default - contents: | - [settings] - arch=x86_64 - build_type=${{ matrix.build_type }} - compiler=${{ matrix.compiler }} - compiler.cppstd=${{ matrix.cppstd }} - compiler.version=${{ matrix.compiler_version }} - os=${{ runner.os }} - compiler.libcxx=${{ matrix.libcxx }} - - [conf] - tools.system.package_manager:mode=install - tools.system.package_manager:sudo=True - write-mode: append - - - uses: DamianReeves/write-file-action@master - if: ${{ matrix.os == 'windows-latest' }} - with: - path: ${{ env.CONAN_HOME }}/profiles/default - contents: | - [settings] - arch=x86_64 - build_type=${{ matrix.build_type }} - compiler=${{ matrix.compiler }} - compiler.cppstd=${{ matrix.cppstd }} - compiler.version=${{ matrix.compiler_version }} - os=${{ runner.os }} - compiler.runtime=${{ matrix.compiler_runtime }} - write-mode: append - - - - name: PrintProfile - run: cat ${{ env.CONAN_HOME }}/profiles/default - - - name: list packages - if: ${{ matrix.os == 'ubuntu-latest' }} - run: dpkg --list + - name: Detect Conan profile + run: conan profile detect --force - name: Install Dependencies - run: conan install . --build=missing --output-folder=build + run: > + conan install . + --build=missing + -s build_type=${{ matrix.build_type }} + -s compiler.cppstd=${{ matrix.cppstd }} + -c "tools.cmake.cmaketoolchain:generator=${{ matrix.cmake_generator }}" + -c tools.system.package_manager:mode=install + -c tools.system.package_manager:sudo=True + + - name: Configure + run: cmake --preset ${{ matrix.configure_preset }} -DBUILD_TESTING=ON - name: Build - run: | - cmake -G "${{ matrix.generator }}" -S . -B build -DCMAKE_TOOLCHAIN_FILE="build/build/generators/conan_toolchain.cmake" -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DBUILD_TESTING=ON - cmake --build build --config ${{ matrix.build_type }} --parallel + run: cmake --build --preset ${{ matrix.build_preset }} --parallel - name: Test - run: ctest --build-config ${{ matrix.build_type }} --test-dir build --output-on-failure + run: ctest --test-dir ${{ matrix.build_dir }} --build-config ${{ matrix.build_type }} --output-on-failure + + # Drop sources, build trees and temporaries so only the package binaries are cached. + - name: Clean Conan cache + if: always() + run: conan cache clean "*" --source --build --temp + + - name: Save Conan cache + if: always() + uses: actions/cache/save@v4 + with: + path: ${{ env.CONAN_HOME }} + key: ${{ steps.cache-conan.outputs.cache-primary-key }} diff --git a/src/views/perioddetailproxymodel.h b/src/views/perioddetailproxymodel.h index 334718b..9549c33 100644 --- a/src/views/perioddetailproxymodel.h +++ b/src/views/perioddetailproxymodel.h @@ -8,7 +8,7 @@ class IntervalModel; class PeriodDetailProxyModel : public QSortFilterProxyModel { public: - explicit PeriodDetailProxyModel(QObject *parent = nullptr); + explicit PeriodDetailProxyModel(QObject* parent = nullptr); void set_source_model(IntervalModel* const model); void set_period(const Period& period); From 7b93de0e0d9a2b1ddc386bc5875c2c529085c14a Mon Sep 17 00:00:00 2001 From: pascal Date: Fri, 28 Aug 2026 20:52:30 +0200 Subject: [PATCH 11/12] fix conan caching --- .github/workflows/cmake-multi-platform.yml | 24 ++++-- conan.lock | 87 ++++++++++++++++++++++ 2 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 conan.lock diff --git a/.github/workflows/cmake-multi-platform.yml b/.github/workflows/cmake-multi-platform.yml index 6fb2b13..e335a44 100644 --- a/.github/workflows/cmake-multi-platform.yml +++ b/.github/workflows/cmake-multi-platform.yml @@ -39,7 +39,10 @@ jobs: CONAN_HOME: "${{ github.workspace }}/.conan2" runs-on: ${{ matrix.os }} - timeout-minutes: 120 + # Generous because conanfile.py selects non-default Qt options, so ConanCenter has + # no prebuilt binary and a cold cache means building Qt from source. Warm runs are + # minutes; only the first run after a conan.lock change pays this. + timeout-minutes: 300 strategy: # Set fail-fast to false to ensure that feedback is delivered for all matrix combinations. Consider changing this to true when your workflow is stable. @@ -78,17 +81,22 @@ jobs: - name: Install Conan run: python -m pip install --upgrade "conan>=2,<3" - # cache/restore + cache/save rather than actions/cache: conanfile.py pins version - # ranges, so an unchanged recipe can still resolve to new dependencies. A plain - # cache would keep hitting its key and never save the newly built packages. + # Keyed on conan.lock, not conanfile.py: the recipe pins version ranges, so its + # hash says nothing about which dependencies actually get resolved. The lockfile + # pins exact versions and recipe revisions, which makes the key truthful — a hit + # really does mean the cache holds the right packages. + # Refresh it after changing dependencies (run both, second builds on the first): + # conan lock create . -s compiler.cppstd=gnu20 --lockfile-out=conan.lock + # conan lock create . -s os=Windows -s compiler=msvc -s compiler.version=194 \ + # -s compiler.runtime=dynamic -s compiler.cppstd=20 -s arch=x86_64 \ + # --lockfile=conan.lock --lockfile-out=conan.lock - name: Restore Conan cache id: cache-conan uses: actions/cache/restore@v4 with: path: ${{ env.CONAN_HOME }} - key: conan-${{ runner.os }}-${{ matrix.build_type }}-${{ matrix.cppstd }}-${{ hashFiles('conanfile.py') }}-${{ github.run_id }} + key: conan-${{ runner.os }}-${{ matrix.build_type }}-${{ matrix.cppstd }}-${{ hashFiles('conan.lock') }} restore-keys: | - conan-${{ runner.os }}-${{ matrix.build_type }}-${{ matrix.cppstd }}-${{ hashFiles('conanfile.py') }}- conan-${{ runner.os }}-${{ matrix.build_type }}-${{ matrix.cppstd }}- - name: Detect Conan profile @@ -98,6 +106,7 @@ jobs: run: > conan install . --build=missing + --lockfile=conan.lock -s build_type=${{ matrix.build_type }} -s compiler.cppstd=${{ matrix.cppstd }} -c "tools.cmake.cmaketoolchain:generator=${{ matrix.cmake_generator }}" @@ -118,8 +127,9 @@ jobs: if: always() run: conan cache clean "*" --source --build --temp + # Only on a miss: an exact key hit means the cache already holds these packages. - name: Save Conan cache - if: always() + if: always() && steps.cache-conan.outputs.cache-hit != 'true' uses: actions/cache/save@v4 with: path: ${{ env.CONAN_HOME }} diff --git a/conan.lock b/conan.lock new file mode 100644 index 0000000..97efcf1 --- /dev/null +++ b/conan.lock @@ -0,0 +1,87 @@ +{ + "version": "0.5", + "requires": [ + "zstd/1.5.7#b68ca8e3de04ba5957761751d1d661f4%1760955092.069", + "zlib/1.3.2#1cb806da49011867778ffb6ac7190fcb%1777558780.503", + "xz_utils/5.8.3#a8432fead347c69d8b2737c35f936132%1775752656.4", + "xorg/system#87ea3cbfa907ff490f4ece2f5fa3dbb9%1781197504.821", + "xkeyboard-config/system#74aea985b91f987b93b220b44963d7ed%1755758035.041", + "xkbcommon/1.5.0#0c9c2e7bc05bda7b40ebd8d7fe6b5830%1783532489.642", + "wayland/1.24.0#ac40d5647b1ce948e185c1a849f63cb6%1779188470.635", + "sqlite3/3.53.4#89fcf5cda598966acb7f3e185b19c58d%1785239491.603", + "spdlog/1.17.0#bcbaaf7147bda6ad24ffbd1ac3d7142c%1767636069.964", + "qt/6.11.1#b4b7bf63674c82b58b825ffca311f656%1786433220.722", + "pcre2/10.42#a0b25dffca18f5987304619e9c93fb44%1783075188.143", + "opengl/system#cfcf523b9d2bad75cbf377f56562634c%1762526044.744", + "libxml2/2.15.3#3406c6d8d327e9b84c0c894feb859514%1777886326.291", + "libselinux/3.6#5a78ff6ae5034eeaac8da723361a8ce4%1717655459.344", + "libpng/1.6.58#19cb72905ae54f54948401f753faa2c1%1776606503.628", + "libmount/2.39.2#64850679fa9d5ead878e5076b0cab2a8%1705591482.625", + "libiconv/1.17#9923bc6dc6f106646d6967e0039a5ada%1774021608.288", + "libgettext/0.22#b09eea019e19b9b9c46d8f1da7d75444%1765809130.834", + "libffi/3.4.8#a045c00fb26779635e3bed40e80c5254%1753360042.396", + "icu/78.2#069e064477b2d48b2215d35ff22de3ce%1786358638.432", + "harfbuzz/12.3.0#a5fef061896cd5c30e9a17c9d7920ad1%1768584500.506", + "glib/2.86.5#2886b0810a99a7d54b313db5d214f221%1783335578.429", + "freetype/2.14.3#4ee27b7918b546a96d7e6898e3a02b34%1774443541.191", + "fontconfig/2.15.0#5d752580de3d735bcdd66db6b7ef4fb2%1768840808.67", + "fmt/12.1.0#6baf0fb8351783472b94ee6e36232391%1785754553.994", + "expat/2.8.3#625b7dbfb3a1ee8b8b5adbb68bfe6f29%1786446698.492", + "elfutils/0.190#ce105950b58f2ec61fb053bb743ff505%1785239818.09", + "double-conversion/3.3.0#de1c45766e70b79dbd9bf744c7b2a89a%1785830926.29", + "bzip2/1.0.8#c470882369c2d95c5c77e970c0c7e321%1762886692.465", + "brotli/1.1.0#3f631ef77008f7b5eb388780116371a3%1764862343.045" + ], + "build_requires": [ + "zstd/1.5.7#b68ca8e3de04ba5957761751d1d661f4%1760955092.069", + "zlib/1.3.2#1cb806da49011867778ffb6ac7190fcb%1777558780.503", + "xz_utils/5.8.3#a8432fead347c69d8b2737c35f936132%1775752656.4", + "xorg/system#87ea3cbfa907ff490f4ece2f5fa3dbb9%1781197504.821", + "xkeyboard-config/system#74aea985b91f987b93b220b44963d7ed%1755758035.041", + "xkbcommon/1.5.0#0c9c2e7bc05bda7b40ebd8d7fe6b5830%1783532489.642", + "wayland-protocols/1.45#79bbdf84ebb0fcc3353686529e0a2322%1757598410.756", + "wayland/1.24.0#ac40d5647b1ce948e185c1a849f63cb6%1779188470.635", + "sqlite3/3.53.4#89fcf5cda598966acb7f3e185b19c58d%1785239491.603", + "qt/6.11.1#b4b7bf63674c82b58b825ffca311f656%1786433220.722", + "pkgconf/2.5.1#93c2051284cba1279494a43a4fcfeae2%1757684701.089", + "pkgconf/2.2.0#4ac315b50ef734072b00ff3aacbf52bf%1755505628.021", + "pcre2/10.42#a0b25dffca18f5987304619e9c93fb44%1783075188.143", + "openssl/3.6.4#bc460bf37686ccae86a15bfdce71be11%1787920038.176", + "opengl/system#cfcf523b9d2bad75cbf377f56562634c%1762526044.744", + "odbc/2.3.11#e3777dc08fe1a8f085332d25213fa123%1772707601.721", + "ninja/1.13.2#c8c5dc2a52ed6e4e42a66d75b4717ceb%1764096931.974", + "meson/1.10.2#9d2d10681fe7fe61c788c58626c89b25%1775558003.754", + "md4c/0.5.2#3d7106721e458f9f799b87d4d50d02e0%1746796758.933", + "m4/1.4.19#1727f439cf74e83826ec96d0b4904eee%1784541921.659", + "lz4/1.9.4#a2f02ad3908618757c68d3618256a196%1775037236.076", + "libxml2/2.15.3#3406c6d8d327e9b84c0c894feb859514%1777886326.291", + "libtool/2.4.7#14e7739cc128bc1623d2ed318008e47e%1755679003.847", + "libselinux/3.6#5a78ff6ae5034eeaac8da723361a8ce4%1717655459.344", + "libpq/17.7#d1ff57ca38fa1cd2c9368643dcc6ac7a%1783436672.112", + "libpng/1.6.58#19cb72905ae54f54948401f753faa2c1%1776606503.628", + "libmount/2.39.2#64850679fa9d5ead878e5076b0cab2a8%1705591482.625", + "libiconv/1.17#9923bc6dc6f106646d6967e0039a5ada%1774021608.288", + "libffi/3.4.8#a045c00fb26779635e3bed40e80c5254%1753360042.396", + "icu/78.2#069e064477b2d48b2215d35ff22de3ce%1786358638.432", + "harfbuzz/12.3.0#a5fef061896cd5c30e9a17c9d7920ad1%1768584500.506", + "gperf/3.1#a7afdf8f7cccdc2dcd4d962370c33d4f%1755780571.156", + "gnu-config/cci.20210814#466e9d4d7779e1c142443f7ea44b4284%1762363589.329", + "glib/2.86.5#2886b0810a99a7d54b313db5d214f221%1783335578.429", + "gettext/0.22.5#2f6edba611e8db1a8a18488b116e8c2a%1765299121.036", + "freetype/2.14.3#4ee27b7918b546a96d7e6898e3a02b34%1774443541.191", + "fontconfig/2.15.0#5d752580de3d735bcdd66db6b7ef4fb2%1768840808.67", + "flex/2.6.4#efa781fc5088b47c895bd4eef6911f2e%1761560242.855", + "expat/2.8.3#625b7dbfb3a1ee8b8b5adbb68bfe6f29%1786446698.492", + "elfutils/0.190#ce105950b58f2ec61fb053bb743ff505%1785239818.09", + "double-conversion/3.3.0#de1c45766e70b79dbd9bf744c7b2a89a%1785830926.29", + "cmake/4.4.2#8a0d360635c870b1d5c675489ee25074%1786458127.662", + "cmake/3.31.12#173a926abc2b77f03c826b6fd6539426%1779785723.158", + "bzip2/1.0.8#c470882369c2d95c5c77e970c0c7e321%1762886692.465", + "brotli/1.1.0#3f631ef77008f7b5eb388780116371a3%1764862343.045", + "bison/3.8.2#c3490cbe0078b6fd3eb4cf5ed64144dc%1755688988.644", + "automake/1.16.5#b91b7c384c3deaa9d535be02da14d04f%1755524470.56", + "autoconf/2.71#51077f068e61700d65bb05541ea1e4b0%1731054366.86" + ], + "python_requires": [], + "config_requires": [] +} From 1b404862bdf8836e0a94ae06592615dda80a6880 Mon Sep 17 00:00:00 2001 From: pascal Date: Fri, 28 Aug 2026 20:57:16 +0200 Subject: [PATCH 12/12] bump msvc --- .github/workflows/cmake-multi-platform.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/cmake-multi-platform.yml b/.github/workflows/cmake-multi-platform.yml index e335a44..3d499e2 100644 --- a/.github/workflows/cmake-multi-platform.yml +++ b/.github/workflows/cmake-multi-platform.yml @@ -53,14 +53,14 @@ jobs: - os: ubuntu-latest build_type: Release cppstd: gnu20 - cmake_generator: Ninja + generator_conf: -c tools.cmake.cmaketoolchain:generator=Ninja configure_preset: conan-release build_preset: conan-release build_dir: build/Release - os: windows-latest build_type: Release cppstd: 20 - cmake_generator: Visual Studio 17 2022 + generator_conf: '' configure_preset: conan-default build_preset: conan-release build_dir: build @@ -87,7 +87,7 @@ jobs: # really does mean the cache holds the right packages. # Refresh it after changing dependencies (run both, second builds on the first): # conan lock create . -s compiler.cppstd=gnu20 --lockfile-out=conan.lock - # conan lock create . -s os=Windows -s compiler=msvc -s compiler.version=194 \ + # conan lock create . -s os=Windows -s compiler=msvc -s compiler.version=195 \ # -s compiler.runtime=dynamic -s compiler.cppstd=20 -s arch=x86_64 \ # --lockfile=conan.lock --lockfile-out=conan.lock - name: Restore Conan cache @@ -109,7 +109,7 @@ jobs: --lockfile=conan.lock -s build_type=${{ matrix.build_type }} -s compiler.cppstd=${{ matrix.cppstd }} - -c "tools.cmake.cmaketoolchain:generator=${{ matrix.cmake_generator }}" + ${{ matrix.generator_conf }} -c tools.system.package_manager:mode=install -c tools.system.package_manager:sudo=True