Skip to content

feat: Add grouplabel support with subscribable mixin #847

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

Merged
merged 3 commits into from
Jul 26, 2019
Merged

feat: Add grouplabel support with subscribable mixin #847

merged 3 commits into from
Jul 26, 2019

Conversation

sidisel-albertolopez
Copy link
Contributor

@sidisel-albertolopez sidisel-albertolopez commented Jul 25, 2019

This is a fix to support group labels. It is a fix for #698. This pull request is like the one published recently by @sathieu #845 with a minor extra change to support subscribable features for GroupLabels. I have used it to programmatically work on group labels for a while now with no issues so far.

This patch extends python-gitlab to support group labels. Take into account that Gitlab GroupLabels API V4 has currently some shortcomings due to inheritance of group labels from parent groups. For instance, the label id is not used to identify a group label in the API, which is a problem as you may retrieve more than one label with the same name and there is no way to distinguish among them, the API ignores the id field. This is a shortcoming of Gitlab API V4, not python-gitlab and it has been reported in gitlab-ce#62322. So the API is expected to change slightly soon to fix it.

Fixes: #698
See also: #845

@sidisel-albertolopez sidisel-albertolopez changed the title Feat/grouplabels Add grouplabel support with subscriptable mixing Jul 25, 2019
@sidisel-albertolopez sidisel-albertolopez changed the title Add grouplabel support with subscriptable mixing feat: Add grouplabel support with subscriptable mixing Jul 25, 2019
@knackaron
Copy link

Verified functionality patched on top of 1.9.0:

gl = gitlab.Gitlab.from_config('dev') 
gl.groups.list()[0].labels.list()
[<GroupLabel name:ztest>]

@sidisel-albertolopez
Copy link
Contributor Author

sidisel-albertolopez commented Jul 25, 2019

I am working in extending the automated testing to include group labels. This is the code I would like to add to tools/python_test_v4.py, not tested yet thou.

# group labels
group1.labels.create({"name": "foo", "description": "bar", "color": "#112233"})
g_l = group1.labels.get("foo")
assert g_l.description == "bar"
g_l.desc = "baz"
g_l.save()
g_l = group1.labels.get("foo")
assert g_l.description == "baz"
assert len(group1.labels.list()) == 1
g_l.delete()
assert len(group1.labels.list()) == 0

Or in patch format:

diff --git a/tools/python_test_v4.py b/tools/python_test_v4.py
index a00ae29..28e5e06 100644
--- a/tools/python_test_v4.py
+++ b/tools/python_test_v4.py
@@ -337,6 +337,18 @@ assert len(group1.variables.list()) == 1
 g_v.delete()
 assert len(group1.variables.list()) == 0
 
+# group labels
+group1.labels.create({"name": "foo", "description": "bar", "color": "#112233"})
+g_l = group1.labels.get("foo")
+assert g_l.description == "bar"
+g_l.description = "baz"
+g_l.save()
+g_l = group1.labels.get("foo")
+assert g_l.description == "baz"
+assert len(group1.labels.list()) == 1
+g_l.delete()
+assert len(group1.labels.list()) == 0
+
 # hooks
 hook = gl.hooks.create({"url": "http://whatever.com"})
 assert len(gl.hooks.list()) == 1

@max-wittig
Copy link
Member

@sidisel-albertolopez Thanks for the contribution! Could you add the proposed tests to it?

@sathieu
Copy link
Contributor

sathieu commented Jul 26, 2019

@sidisel-albertolopez Could you rebase? Could you sort gitlab/v4/objects.py (i.e. GroupLabel before GroupMember)?

@sidisel-albertolopez
Copy link
Contributor Author

@sathieu Rebased to latest master, sorted objects alphabetically, please confirm its ok

@max-wittig Tests added

@max-wittig
Copy link
Member

@sidisel-albertolopez Thank you! 👍

Would you mind to also add tests for the CLI?

@sidisel-albertolopez
Copy link
Contributor Author

@max-wittig I don't see any tests for groups or labels, not at least in tools/cli_test_v4.sh. Any guidance?

@sidisel-albertolopez
Copy link
Contributor Author

@max-wittig Added some cli tests, had to create required simple group create and update test. I still cannot verify if these tests are correct or not. Any help appreciated

@sidisel-albertolopez
Copy link
Contributor Author

sidisel-albertolopez commented Jul 26, 2019

@max-wittig CLI tests are now working ok, tested with tox -e cli_func_v4 on development environment.

To have CLI tests working I had to create a test group. As no tests where available to test groups or project labels, I have added the following CLI tests on tools/cli_test_v4.sh:

  • group create
  • group update
  • project label create
  • project label list
  • project label update
  • project label delete
  • group label create
  • group label list
  • group label update
  • group label delete
  • group delete

While testing for group labels I have found a bug in CLI project-label update. As both UPDATE and DELETE HTTP methods in GitLab API V4 for ProjectLabel and GroupLabel do not use (yet) a label id parameter and use only the name, it is required to have an update method in GroupLabelManager and ProjectLabelManager similar to the delete method found in ProjectLabelManager which uses name parameter instead of id.

I have added the update method to ProjectLabelManager and GroupLabelManager so that they ensure the id field is None and the name is explicitly added to the new_data dictionary before calling the UpdateMixin update method. This is a fix specific to the ProjectLabel and GroupLabel API which is subject to change (it may become unnecessary) once the gitlab-ce#62322 issue is merged.

The fix the to ProjectLabelManager update method may be better suited for a different pull-request than this one, as it is not GroupLabel related.

@sidisel-albertolopez sidisel-albertolopez changed the title feat: Add grouplabel support with subscriptable mixing feat: Add grouplabel support with subscriptable mixin Jul 26, 2019
@sidisel-albertolopez sidisel-albertolopez changed the title feat: Add grouplabel support with subscriptable mixin feat: Add grouplabel support with subscribable mixin Jul 26, 2019
@max-wittig
Copy link
Member

@sidisel-albertolopez That's pretty awesome! 👏 Thanks for the contribution!

@max-wittig max-wittig merged commit edb3359 into python-gitlab:master Jul 26, 2019
@sathieu
Copy link
Contributor

sathieu commented Jul 26, 2019

@sidisel-albertolopez Impressive work. Thanks!

@MatthieuPERIN
Copy link

MatthieuPERIN commented Jun 12, 2020

Hello, if I may propose something:
It would be nice also to have a get() method based on label id or name (like what is proposed for projects) because working on the bare result fromlist() command is kinda frustrating.

Something like grp.labels.get(id) or prj.labels.get(id)

Thanks for this amazing tool :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot create/update group labels
5 participants