Skip to content
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

[BUG] addLabels not a function #1314

Closed
ivankatliarchuk opened this issue Sep 25, 2022 · 10 comments · Fixed by #1353
Closed

[BUG] addLabels not a function #1314

ivankatliarchuk opened this issue Sep 25, 2022 · 10 comments · Fixed by #1353
Labels

Comments

@ivankatliarchuk
Copy link
Contributor

Describe the bug

Probably not a bug, I might be missing something.
According to this PR, following should just work

// Add labels
danger.gitlab.api.addLabels('Label 1', 'Label 2');

However its not a case.

To Reproduce
Steps to reproduce the behavior:

const { danger } = require('danger');
danger.gitlab.api.addLabels('Label 1', 'Label 2');

TypeError: addLabels is not a function at beforeCompletion (dangerfile.js:2:9)

For ts, same result

import { danger } from "danger";
danger.gitlab.api.addLabels('Label 1', 'Label 2');

TypeError: addLabels is not a function

Expected behavior
It should update labels.

Current approach

const addLabels = async (prId, input) => {
  const mrLabels = danger.gitlab.mr.labels
  const changes = [...mrLabels, ...input.filter(el => !mrLabels.includes(el, 0))];
  if (mrLabels.length != changes.length) {
    const mr = await danger.gitlab.api.MergeRequests.edit(repo, prId, { labels: [...mrLabels, ...input] })
    console.log(`updated labels '${changes}' for mr '${prId}'`)
  }
}

Labels updated.

Screenshots
Screenshot 2022-09-25 at 13 21 41

Your Environment

software version
danger.js 11.1.2
node v14.16.1) and v18.9.0
npm (npm@6.14.12 and (npm v8.19.1)
Operating System Macos and Ubuntu(Docker)
@orta
Copy link
Member

orta commented Sep 25, 2022

I don't think those are on that object (I think the PR had the wrong text) try without the .api (or check the typescript types

@ivankatliarchuk
Copy link
Contributor Author

ivankatliarchuk commented Oct 1, 2022

I went through the code.
danger.gitlab.api actually returns InstanceType<typeof Gitlab>. Not sure if was it intentional or not, but as a result, functions addLabels and removeLabels not used anywhere in the library, would assume not in use outide of it too.

addLabels = async (...labels: string[]): Promise<boolean> => {

Does it sound allright if I make them available through danger.gitlab.uits.addLabels ? Other option is to remove them, as its just redundant/untested piece of code.

@orta
Copy link
Member

orta commented Oct 1, 2022

I think it is available as danger.gitlab.addLabels judging on the code

@ivankatliarchuk
Copy link
Contributor Author

ivankatliarchuk commented Oct 1, 2022

console.log(Object.getOwnPropertyNames(danger.gitlab))
console.log(Object.getOwnPropertyNames(danger.gitlab).filter(function (p) {
  return typeof danger.gitlab[p] === 'function';
}));

output

[ 'metadata', 'mr', 'commits', 'approvals', 'utils', 'api' ]
[]

Not the end of the world

@stillborn333

This comment was marked as spam.

@ivankatliarchuk
Copy link
Contributor Author

Looks like there is an inconstitency in the code e.g. Github #1008

danger.github.utils.createOrAddLabel

While Gitlab does not have anything like danger.gitlab.uits.createOrAddLabel

@glensc
Copy link
Contributor

glensc commented Feb 7, 2023

This should add the two methods under utils:

but imho, this is a mess, why need to map, why not expose the whole "api"?

@glensc
Copy link
Contributor

glensc commented Feb 7, 2023

The actual breakage was from this commit by @orta:

Find source/platforms/GitLab.ts file in the diff because the commit is huge

@glensc
Copy link
Contributor

glensc commented Feb 7, 2023

That commit links to #1192 that has no context, at all, but 250 commits spanning multiple releases and merges or what the "improper" was. imho now the dsl exposed is improper raw GitLab class import { Gitlab } from "@gitbeaker/node" rather danger-js GitLabAPI class

@glensc
Copy link
Contributor

glensc commented Feb 8, 2023

Changelog is recorded in such way that the same person added addLabels/removeLabels and broke them in subsequent pr?

# 10.6.6
Fix for supporting Bitbucket Server personal repositories
GitLab: Added GitLabApi to danger.gitlab.api. - [@shyim]
GitLab: Added label helper functions to danger.gitlab.api.addLabels and danger.gitlab.api.removeLabels. - [@shyim]

cc @shyim

@orta orta closed this as completed in #1353 Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants