-
Notifications
You must be signed in to change notification settings - Fork 130
Adding in support to get attributes by attribute #61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@necrolyte2 Couldn't we have duplicates, for example two records with If thats the case, it feels like this should return a list, and maybe be called 'get_all_by_attribute`. |
Good point. I'll change it and fix the build error too. |
Changes Unknown when pulling f82743c on necrolyte2:master into * on maxtepkeev:master*. |
redmine/managers.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably just return an empty list rather than raise an exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So just let the caller decide if it is an exception if the attribute does not exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
N/m I see what you mean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think that makes sense; if no items are found that match the criteria, just return an empty record set.
…t return an Exception if the result set is empty
Changes Unknown when pulling 584b524 on necrolyte2:master into * on maxtepkeev:master*. |
I don't think that it should live in a ResourceManager. I believe that ResourceSet is the correct place for this method, otherwise you're hardcoding it to only work with |
Makes sense. I won't be able to get to changing that until next week, so if you get time and want to do it go for it. |
I'm closing this pull request, because I've implemented a little bit more powerful Django-like lookup system for ResourceSet's issues = redmine.issue.filter(project_id='foobar').filter(status__id=(2, 7), tracker__id=5, done_ratio=50) i.e. the first call to I'll push the commits with this functionality a little bit later, because I want to rewrite a documentation a little bit and write some more tests for this. |
Maybe I am doing it wrong, but there are a lot of cases that I want to get say a tracker or custom_field by name. Since the Redmine API does not support this it makes it difficult and you have to essentially fetch all records and then filter them down manually.
This should essentially do that process instead of having the api user write their own function
Does it make sense to support a non-lazy operation though?