I was going to work on #4663 and found some problems on the get endpoint, so fix them first:
And fixes #4663 after
:thumbsup:
~~Don't merge this yet, I wanna check some more things :)~~
rebased onto 63e3e4a2f94293271a943c98d12fadbc9bb37d0f
rebased onto 280da6908ca1304a7052f6f037e54a3c348e9f75
Should I remove the ENABLE_TICKETS check? tags can be used on pull requests too...
1 new commit added
Fix doc typo
rebased onto befd3f0c180a461a0bd72510694d146ef27c2cc4
forms: tag must be set on *IssueTagForm
api docs: move project tags docs to project section
remove_tags query: use reusable remove_tags_obj() for issue|pull_request untagging
8 new commits added
api/project: get project tag endpoint
api/project: patch project tag endpoint
forms: edit issue tag form
api/project: delete project tag endpoint
api.utils: _get_project_tag helper method
api/project: new project tag endpoint
tests: move project tags tests to their own file
models: to_json method on TagColored
14 new commits added
tests: move project tags test to project tests file
Some modifications on project tag get api endpoint:
pretty please pagure-ci rebuild
Not sure but you only add a tag here, not an issue. Is the comment wrong ?
wrong doc string
indeed, I added one but it was not necessary and removed it after, and did not update the comment. I'll fix it
So that looks good to me as well ! :thumbsup:
rebased onto 91bd166304ac087795317d2386b520a56d988455
Hm, I need to check this more closely but the changes make me wonder if this is backward compatible?
Current API endpoint has not been touched.
AFAIK, The possible non backward compatible change is that now tag.tag is mandatory while it was an optional one, but on the ui part I have been looking tag.tag was mandatory on html forms, so yeah, the POST call is not fully backward compatible, but the overall result is.
tag.tag
Anyhow, I have to fix the pr and resolve conflicts with #4669
rebased onto f11c9717c7a94e2f43a0c4efd6f462235b6ba3d5
Rebased and ready for review. I ended stacking a bunch of commits so feel free to review commit by commit :)
Tests will fail on pip container due to the pygit2 thing, but it already passed on f29
@pingou you were right, there are some backward incompatible changes:
f11c9717c7a94e2f43a0c4efd6f462235b6ba3d5 : the error output on /api/0//tags has changed the format. Previously it had a custom output when repo was not found, now uses the same helper method and same output that the rest of endpoints uses for this.
e881c7f096d8520ef64f0380e62a2b9bd6ebbc71: this modifies the form's mandatory fields, so the POST endpoint requirements had changed. But in frontend side this field was already mandatory, so the ui is backward compatible.
1e2e964d002967c82319b7cc813085504224e2bc: this produces ev and log events for each of the untagged issues|pull_requests when someone removes a tag that is actively used on issues|pull requests. improvement, but could be a bit spammer if someone removes a tag that had been used on 1k issues :)
The rest of the changes are new features or backward compatible afaik.
Arg, non-backward compatible changes are always tricky to handle :(
f11c971 : the error output on /api/0//tags has changed the format. Previously it had a custom output when repo was not found, now uses the same helper method and same output that the rest of endpoints uses for this.
I'm kinda of ok with this one as that endpoint was the exception more than the rule
e881c7f: this modifies the form's mandatory fields, so the POST endpoint requirements had changed. But in frontend side this field was already mandatory, so the ui is backward compatible.
Shouldn't this be documented in the API as mandatory field then? And wasn't the fact that it was optional used as a way to remove a tag?
1e2e964: this produces ev and log events for each of the untagged issues|pull_requests when someone removes a tag that is actively used on issues|pull requests. improvement, but could be a bit spammer if someone removes a tag that had been used on 1k issues :)
This is backward compatible but indeed, may lead to some notifications flooding :s
Arg, non-backward compatible changes are always tricky to handle :( f11c971 : the error output on /api/0//tags has changed the format. Previously it had a custom output when repo was not found, now uses the same helper method and same output that the rest of endpoints uses for this. I'm kinda of ok with this one as that endpoint was the exception more than the rule
e881c7f: this modifies the form's mandatory fields, so the POST endpoint requirements had changed. But in frontend side this field was already mandatory, so the ui is backward compatible. Shouldn't this be documented in the API as mandatory field then? And wasn't the fact that it was optional used as a way to remove a tag?
I'll recheck what's going on the ui part with this since I don't remember
1e2e964: this produces ev and log events for each of the untagged issues|pull_requests when someone removes a tag that is actively used on issues|pull requests. improvement, but could be a bit spammer if someone removes a tag that had been used on 1k issues :) This is backward compatible but indeed, may lead to some notifications flooding :s
The main reason for this is sending ev events so if you're viewing issue #n with tag foo and I remove the tag from the project then you receive the ev event on the same way that you would receive it if I just untag the issue. I'll open an issue about this and get the commit out from this pr, we can handle that on a different phase
rebased onto 91c16e7882876b31347ad73779b75d112847d1fc
rebased onto 87f4a6baa85e04813cc14f0079e1fc3c98f3a71d
1e2e964 removed from pull request. Something is wrong for el7, I'll fix that before continuing with the rest of modifications
10 new commits added
I'll not have time to fix the issues with patch endpoint during next days, and that's not necessary to unblock @fbo , so I made some changes on the commit stack:
@jlanda can we land this? It's been lingering for too long... :'(
Yes. I removed the patch endpoint, we can land it later
rebased onto 41d733d417f4065f7c6d26e5807c4142901f34d1
Indentation issue on this one. Also note for these two lines that the | in the description should not be needed since it is a single line.
|
One tiny change and let's get this in :)
Done.
Let's be consistent and remove the | here as well :)
rebased onto 2dba705f2e0a9a668241a3b54e3055334401b922
Pull-Request has been merged by pingou
I was going to work on #4663 and found some problems on the get endpoint, so fix them first:
And fixes #4663 after