GothenburgBitFactory / bugwarrior

Pull github, bitbucket, and trac issues into taskwarrior
http://pypi.python.org/pypi/bugwarrior
GNU General Public License v3.0
732 stars 209 forks source link

Adds support for Nextcloud Deck, a Kanban-Style task management tool … #954

Closed lenalebt closed 1 year ago

lenalebt commented 1 year ago

…integrated with Nextcloud

It already works more or less, but I'm not very used to writing python code. I'm open for suggestions on how to improve stuff :-). I think functionality-wise it should all be there already.

A bit of context: I'm using nextcloud deck for managing stuff in the household together with my wife, but I'd like to get those tasks into my taskwarrior as well ;-). I currently plan to do this regularly in the upcoming time.

lenalebt commented 1 year ago

Thanks for the review, highly appreciated and very good points raised :smile: !

I would love to test the filter_repos, but had some trouble setting the config values needed for that. Is there an easy way to do so? Example code:

def test_filter_repos_include(self):
    self.service.config['deck.include_board_ids'] = ['5']
    self.assertTrue(self.service.filter_repos({'title': 'testboard', 'id': 5}))
    self.assertFalse(self.service.filter_repos({'title': 'testboard', 'id': 6}))

def test_filter_repos_exclude(self):
    self.service.config['deck.exclude_board_ids'] = ['5']
    self.assertFalse(self.service.filter_repos({'title': 'testboard', 'id': 5}))
    self.assertTrue(self.service.filter_repos({'title': 'testboard', 'id': 6}))

I think I'm just holding it wrong. It's not that I think those are absolutely crucial tests, but it would be nice to have it working somehow.

ryneeverett commented 1 year ago

I would love to test the filter_repos, but had some trouble setting the config values needed for that. Is there an easy way to do so?

We don't seem to have the test infrastructure to do this as trivially as would be ideal, but you can check out test_gitlab.py:TestGitlabService for an example. If you copied the setUp method and altered it to have the minimum configuration for deck, you could then test the configurations with:

def test_filter_repos_include(self):
    self.config.set('myservice', 'deck.include_board_ids', '5')
    self.assertTrue(self.service.filter_repos({'title': 'testboard', 'id': 5}))
    self.assertFalse(self.service.filter_repos({'title': 'testboard', 'id': 6}))

def test_filter_repos_exclude(self):
    self.config.set('myservice', 'deck.exclude_board_ids', '5')
    self.assertFalse(self.service.filter_repos({'title': 'testboard', 'id': 5}))
    self.assertTrue(self.service.filter_repos({'title': 'testboard', 'id': 6}))
lenalebt commented 1 year ago

I tried doing that, but I had some trouble initially with the setup because all the mocking works differently over there. I worked it out - thanks for your help :-).

Besides that I fixed the validation problems around line lengths. I'm happy with it now, if you're fine with it it can be merged.

ryneeverett commented 1 year ago

@lenalebt I hope you don't mind I pushed a couple commits to your branch to fix CI and implement conditional annotation fetching. Setting annotation_comments = False in the [general] section of your bugwarriorrc should satisfy your needs.

lenalebt commented 1 year ago

Oh wow, no problem, thank you! I had a little bit too much on my table lately.

So, I'm fine with merging now, I'm using it on a regular basis and don't see problems so far. Anything I can do now in addition to get it merged :-)?

ryneeverett commented 1 year ago

So, I'm fine with merging now, I'm using it on a regular basis and don't see problems so far. Anything I can do now in addition to get it merged :-)?

Nope, I was just waiting for your approval of the changes. :)