Skip to content

feat: completion of parent task will mark subtasks as done #116

Merged
snehmatic merged 5 commits into
snehmatic:mainfrom
jakubdomanski:feature/111-parent-task-completes-subtasks
May 14, 2026
Merged

feat: completion of parent task will mark subtasks as done #116
snehmatic merged 5 commits into
snehmatic:mainfrom
jakubdomanski:feature/111-parent-task-completes-subtasks

Conversation

@jakubdomanski
Copy link
Copy Markdown
Contributor

@jakubdomanski jakubdomanski commented May 14, 2026

Closes: #111
Soooo, I've implemented subtask completion when parent task is completed. That's the good part.
Too much code needed change to be more readible, so i made the change and didn't refactored anything.

Maybe let's talk about this here? You can move some to issues, and i can work on some.

My proposition:

  1. config.UserConfig - imo this should be singleton, like config.Config.
  2. task.Service - user config should be treated as dependency, not passing individual scores to methods e.g. func NewService(db *gorm.DB, uc *config.UserConfig) *Service {.
  3. Errors should be at package lvl = easier to compare in tests.
    e.g.
var (
    ErrSubTaskNotFound = errors.New("subtask not found")
    ErrTaskNotFound    = errors.New("task not found")
)
  1. Magic strings.
task.Status = "completed"

vs

const (
    StatusCompleted = "completed"
    StatusPending   = "pending"
)
  1. Dependency Injection.

  2. Repository pattern for separation of duties, now task have share db and business logic.

  3. Broken tests on fresh fork make test (without any changes)

% make test
>  Running tests...
?       github.com/snehmatic/mindloop   [no test files]
✅ User config written to YAML successfully
--- FAIL: TestHabitFlow (0.00s)
    handlers_test.go:129: Log Habit failed/redirected wrong: /habits?success=true

2026/05/14 01:22:55 ./mindloop/internal/core/summary/summary.go:43 SQL logic error: no such table: Task (1)
[0.073ms] [rows:0] SELECT count(*) FROM `Task` WHERE (Status = "completed" AND UpdatedAt >= "2026-05-07 01:22:55.027" AND UpdatedAt <= "2026-05-14 01:22:55.027") AND `Task`.`DeletedAt` IS NULL
FAIL
FAIL    github.com/snehmatic/mindloop/api/v1    0.594s
?       github.com/snehmatic/mindloop/cmd/cli   [no test files]
?       github.com/snehmatic/mindloop/cmd/server        [no test files]
?       github.com/snehmatic/mindloop/db        [no test files]
?       github.com/snehmatic/mindloop/internal/config   [no test files]
?       github.com/snehmatic/mindloop/internal/core/ai  [no test files]
ok      github.com/snehmatic/mindloop/internal/core/backup      (cached)
ok      github.com/snehmatic/mindloop/internal/core/focus       (cached)
ok      github.com/snehmatic/mindloop/internal/core/habit       (cached)
ok      github.com/snehmatic/mindloop/internal/core/intent      (cached)
ok      github.com/snehmatic/mindloop/internal/core/journal     (cached)
?       github.com/snehmatic/mindloop/internal/core/motivation  [no test files]
ok      github.com/snehmatic/mindloop/internal/core/note        (cached)
ok      github.com/snehmatic/mindloop/internal/core/points      (cached)
ok      github.com/snehmatic/mindloop/internal/core/quest       (cached)
?       github.com/snehmatic/mindloop/internal/core/routine     [no test files]
ok      github.com/snehmatic/mindloop/internal/core/summary     (cached)
?       github.com/snehmatic/mindloop/internal/core/task        [no test files]
?       github.com/snehmatic/mindloop/internal/log      [no test files]
?       github.com/snehmatic/mindloop/internal/server   [no test files]
ok      github.com/snehmatic/mindloop/internal/utils    (cached)
?       github.com/snehmatic/mindloop/models    [no test files]
?       github.com/snehmatic/mindloop/web       [no test files]
FAIL
make: *** [test] Error 1
  1. CI - tests, vet, fmt, should run also outside main imho.
  2. Missing tests for task.

That's all for now.

Added user configuration handling to the task service and updated task completion logic to preload subtasks.
@snehmatic
Copy link
Copy Markdown
Owner

Too much code needed change to be more readible, so i made the change and didn't refactored anything.
Maybe let's talk about this here? You can move some to issues, and i can work on some.
My proposition:

haha yea refactoring has been one of the things I've been putting off just because. idk.

I like your thoroughness @jakubdomanski . Based on your couple propositions:

  1. Point 1: That makes sense. User can set/reset stuff inbetween, we can just update the reference. Sounds good. Singleton for UserConfig #117
  2. Point 2, 3, 4: Agreed! Cleanup and organise common variables and errors #119 - for 3, 4.
  3. Point 5, 6: Could you elaborate? You know what, I can create an issue and you can add there. [Refactor] Code enhancement #118
  4. Point 7: I think my recent changes broke it, will check it out.
  5. Point 8: we have Test running on PRs, others we can add.
  6. Point 9: Good point, it's relatively new so yea. Unit tests for Tasks feature #120

Feel free to comment on the ones you want to work on, I'll leave them alone :P

@jakubdomanski
Copy link
Copy Markdown
Contributor Author

Ad 5. Dependency injection will allow to test this better e.g. injecting logger instead of using logger.Error().Err(err).Msg("Failed to create task") will allow to mock and check for method invokes. This will allow testing of small units of code, without coupling with other units.

Ad 6. Repository pattern - Only business logic will live in service and rest will be injected with appropriate interfaces.
createTask in repository -> award points service (calculate points -> save points in repository) -> return and the service will not depend on db *gorm.DB this change will allow to implement adapters for other databases, you will only need to supply (using e.g. factory pattern) repository that fills repository interface and dont care about underlining implementation detalis.

@jakubdomanski
Copy link
Copy Markdown
Contributor Author

Point 8: we have Test running on PRs, others we can add. - yeah, I meant that on my fork branches no tests were running after commit, so after manual testing i did not know if my code on branch was compliant with linter and so on. I would like to have tests that will tell me that my changes are ready for CR.

@snehmatic
Copy link
Copy Markdown
Owner

Sounds good @jakubdomanski. Thanks again!

The gh action workflow needed approval on fork branch (for first time contribution). That's it.

@snehmatic
Copy link
Copy Markdown
Owner

the test issue should be fixed and in main

@jakubdomanski
Copy link
Copy Markdown
Contributor Author

ok, hopefully today i'll pull your changes to my branch.

on the weekend i can work on #117, #119 and maybe #118, you can assign me if that's ok with you.

@jakubdomanski
Copy link
Copy Markdown
Contributor Author

done,
but i needed to remove error handling for tests only, here see last commit 0efc0bf

@snehmatic snehmatic merged commit b895625 into snehmatic:main May 14, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Completing a Parent Task should mark all sub-tasks as done

2 participants