Skip to content

feat: Implement category support#475

Merged
WhyNotHugo merged 26 commits into
pimutils:mainfrom
0styx0:implement_categories
Jul 5, 2022
Merged

feat: Implement category support#475
WhyNotHugo merged 26 commits into
pimutils:mainfrom
0styx0:implement_categories

Conversation

@0styx0
Copy link
Copy Markdown
Contributor

@0styx0 0styx0 commented Jan 24, 2022 •
edited
Loading

This pr builds on and supersedes #323 to implement full category support using the -c/—category flag.
Solves #10

0styx0 and others added 16 commits January 22, 2022 14:04
@0styx0
feat(categories): added basic category support
386f1d9
@0styx0
feat(categories): added to interactive layer
94f7c08
@0styx0
feat(categories): allow for filtering by multiple categories and fixe…
a1529da 
…d database antipattern
@0styx0
chore: removed unnecessary db fields/code
0500cc5
@0styx0
tests: all tests pass
44355a8
@0styx0
tests(category): created tests
512766e
@0styx0
docs: added category support
a9fb63d
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
6a95923 
for more information, see https://pre-commit.ci
@0styx0
fix: incompatible types
20df442
@0styx0
ci: fix E501 (line too long)
e712327
@0styx0
chore(ci): removed unnecesary lines to increase required code coverage
9e6c9fa
@0styx0
fix: can edit categories in interactive mode
f80d93c
@0styx0
chore: removed unneeded code for code coverage
3a0a699
@0styx0
test: category sql contraint
f044096
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
0738e0a 
for more information, see https://pre-commit.ci
@0styx0
chore: removed unused method, added back drop_tables
16cb941
WhyNotHugo
WhyNotHugo requested changes Jan 27, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@WhyNotHugo WhyNotHugo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing this, this is a pretty tricky one.

I think you're not deleting categories from the categories table when a todo is deleted -- and they might show up for later todos when ids get flushed.

Comment thread todoman/cli.py Outdated
Comment thread todoman/formatters.py Outdated
Comment thread todoman/model.py Outdated
Comment thread todoman/model.py Outdated
Comment thread todoman/model.py Outdated
Comment thread todoman/model.py Outdated
@0styx0
Copy link
Copy Markdown
Contributor Author

0styx0 commented Jan 31, 2022

Thanks for addressing this, this is a pretty tricky one.

I think you're not deleting categories from the categories table when a todo is deleted -- and they might show up for later todos when ids get flushed.

the line FOREIGN KEY(todos_id) REFERENCES todos(id) ON DELETE CASCADE in https://github.com/0styx0/todoman/blob/b2063c84fbcd0fb84d3bb540e661e40f6b505d3b/todoman/model.py#L506 should take care of that. I added a test in 004d812 to make sure, let me know if I missed it though

@0styx0 0styx0 requested a review from WhyNotHugo February 1, 2022 17:00
WhyNotHugo
WhyNotHugo requested changes Feb 3, 2022
View reviewed changes
Comment thread tests/test_cli.py
Comment thread tests/test_model.py
Comment thread tests/test_model.py
Comment thread todoman/cli.py Outdated
@0styx0
fix: added back stricter check
c6af1d7 
adding the [] check to pass tests/test_cli.py::test_edit_move
@0styx0 0styx0 requested a review from WhyNotHugo February 13, 2022 20:33
WhyNotHugo
WhyNotHugo approved these changes Jul 5, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@WhyNotHugo WhyNotHugo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Sorry for the delay, lost sight of this.

@WhyNotHugo WhyNotHugo merged commit 4c1c6a7 into pimutils:main Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@WhyNotHugo WhyNotHugo WhyNotHugo approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@0styx0 @WhyNotHugo