#4664 api modifications for project tags
Merged by pingou. Opened by jlanda.
jlanda/pagure api-tags-namespaces  into  master

Download 4664.patch

I was going to work on #4663 and found some problems on the get endpoint, so fix them first:

  • Add namespace support
  • Move it to project with the rest of project|repo related endpoints
  • Use api_method decortator and _get_repo

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

1 new commit added

  • forms: tag must be set on *IssueTagForm

1 new commit added

  • api docs: move project tags docs to project section

1 new commit added

  • 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

  • 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
  • remove_tags query: use reusable remove_tags_obj() for issue|pull_request untagging
  • api docs: move project tags docs to project section
  • forms: tag must be set on *IssueTagForm
  • tests: move project tags test to project tests file
  • Fix doc typo
  • Some modifications on project tag get api endpoint:

14 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
  • remove_tags query: use reusable remove_tags_obj() for issue|pull_request untagging
  • api docs: move project tags docs to project section
  • forms: tag must be set on *IssueTagForm
  • tests: move project tags test to project tests file
  • Fix doc typo
  • Some modifications on project tag get api endpoint:

14 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
  • remove_tags query: use reusable remove_tags_obj() for issue|pull_request untagging
  • api docs: move project tags docs to project section
  • forms: tag must be set on *IssueTagForm
  • tests: move project tags test to project tests file
  • Fix doc typo
  • Some modifications on project tag get api endpoint:

14 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
  • remove_tags query: use reusable remove_tags_obj() for issue|pull_request untagging
  • api docs: move project tags docs to project section
  • forms: tag must be set on *IssueTagForm
  • tests: move project tags test to project tests file
  • Fix doc typo
  • Some modifications on project tag get api endpoint:

14 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
  • remove_tags query: use reusable remove_tags_obj() for issue|pull_request untagging
  • api docs: move project tags docs to project section
  • forms: tag must be set on *IssueTagForm
  • tests: move project tags test to project tests file
  • Fix doc typo
  • 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

Not sure but you only add a tag here, not an issue. Is the comment wrong ?

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:

14 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
  • remove_tags query: use reusable remove_tags_obj() for issue|pull_request untagging
  • api docs: move project tags docs to project section
  • forms: tag must be set on *IssueTagForm
  • tests: move project tags test to project tests file
  • Fix doc typo
  • Some modifications on project tag get api endpoint:

pretty please pagure-ci rebuild

rebased onto 91bd166304ac087795317d2386b520a56d988455

Hm, I need to check this more closely but the changes make me wonder if this is backward compatible?

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.

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

:thumbsup:

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 f11c9717c7a94e2f43a0c4efd6f462235b6ba3d5

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

  • api/project: get project tag endpoint
  • 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
  • api docs: move project tags docs to project section
  • tests: move project tags test to project tests file
  • Fix doc typo
  • Some modifications on project tag get api endpoint:

10 new commits added

  • api/project: get project tag endpoint
  • 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
  • api docs: move project tags docs to project section
  • tests: move project tags test to project tests file
  • Fix doc typo
  • Some modifications on project tag get api endpoint:

10 new commits added

  • api/project: get project tag endpoint
  • 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
  • api docs: move project tags docs to project section
  • tests: move project tags test to project tests file
  • Fix doc typo
  • Some modifications on project tag get api endpoint:

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:

  • get rid of backward incompatible form changes. better safe than sorry.
  • align new tag api endpoint's doc string with new form.
  • remove the patch endpoint that is failing to validate the form on el7 environment

:thumbsup:

@jlanda can we land this? It's been lingering for too long... :'(

@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 :)

10 new commits added

  • api/project: get project tag endpoint
  • 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
  • api docs: move project tags docs to project section
  • tests: move project tags test to project tests file
  • Fix doc typo
  • Some modifications on project tag get api endpoint:

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.

Done.

Let's be consistent and remove the | here as well :)

10 new commits added

  • api/project: get project tag endpoint
  • 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
  • api docs: move project tags docs to project section
  • tests: move project tags test to project tests file
  • Fix doc typo
  • Some modifications on project tag get api endpoint:

rebased onto 2dba705f2e0a9a668241a3b54e3055334401b922

Pull-Request has been merged by pingou

Metadata