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

Proposal: Organization labels #7406

Closed
mrsdizzie opened this issue Jul 10, 2019 · 5 comments · Fixed by #10814
Closed

Proposal: Organization labels #7406

mrsdizzie opened this issue Jul 10, 2019 · 5 comments · Fixed by #10814
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/enhancement An improvement of existing functionality
Milestone

Comments

@mrsdizzie
Copy link
Member

I'd like to implement labels on an organization level.

I think it should be pretty straight forward to add an org_id column to the label table and have it mostly work minus all interface updates/additions.

The way labels were implemented has them pretty coupled to repositories in code and language (compared to Webhooks, which seem to have been implemented without that limitation in mind and exist across repos and organizations ).

Many label functions currently are of the form GetLabelsByRepoID, getLabelInRepoByName, etc... and most of them rely on having a RepoID passed through.

Just want to get feedback if it is worth refactoring/renaming the following functions to take both a repoID and orgID (one always being nil) where appropriate since most of them would work the same otherwise and we would just want to do a lookup based on repoID OR orgID going forward:

gitea/models/issue_label.go

Lines 155 to 308 in 877df0f

// getLabelInRepoByName returns a label by Name in given repository.
// If pass repoID as 0, then ORM will ignore limitation of repository
// and can return arbitrary label with any valid ID.
func getLabelInRepoByName(e Engine, repoID int64, labelName string) (*Label, error) {
if len(labelName) == 0 {
return nil, ErrLabelNotExist{0, repoID}
}
l := &Label{
Name: labelName,
RepoID: repoID,
}
has, err := e.Get(l)
if err != nil {
return nil, err
} else if !has {
return nil, ErrLabelNotExist{0, l.RepoID}
}
return l, nil
}
// getLabelInRepoByID returns a label by ID in given repository.
// If pass repoID as 0, then ORM will ignore limitation of repository
// and can return arbitrary label with any valid ID.
func getLabelInRepoByID(e Engine, repoID, labelID int64) (*Label, error) {
if labelID <= 0 {
return nil, ErrLabelNotExist{labelID, repoID}
}
l := &Label{
ID: labelID,
RepoID: repoID,
}
has, err := e.Get(l)
if err != nil {
return nil, err
} else if !has {
return nil, ErrLabelNotExist{l.ID, l.RepoID}
}
return l, nil
}
// GetLabelByID returns a label by given ID.
func GetLabelByID(id int64) (*Label, error) {
return getLabelInRepoByID(x, 0, id)
}
// GetLabelInRepoByName returns a label by name in given repository.
func GetLabelInRepoByName(repoID int64, labelName string) (*Label, error) {
return getLabelInRepoByName(x, repoID, labelName)
}
// GetLabelIDsInRepoByNames returns a list of labelIDs by names in a given
// repository.
// it silently ignores label names that do not belong to the repository.
func GetLabelIDsInRepoByNames(repoID int64, labelNames []string) ([]int64, error) {
labelIDs := make([]int64, 0, len(labelNames))
return labelIDs, x.Table("label").
Where("repo_id = ?", repoID).
In("name", labelNames).
Asc("name").
Cols("id").
Find(&labelIDs)
}
// GetLabelInRepoByID returns a label by ID in given repository.
func GetLabelInRepoByID(repoID, labelID int64) (*Label, error) {
return getLabelInRepoByID(x, repoID, labelID)
}
// GetLabelsInRepoByIDs returns a list of labels by IDs in given repository,
// it silently ignores label IDs that do not belong to the repository.
func GetLabelsInRepoByIDs(repoID int64, labelIDs []int64) ([]*Label, error) {
labels := make([]*Label, 0, len(labelIDs))
return labels, x.
Where("repo_id = ?", repoID).
In("id", labelIDs).
Asc("name").
Find(&labels)
}
// GetLabelsByRepoID returns all labels that belong to given repository by ID.
func GetLabelsByRepoID(repoID int64, sortType string) ([]*Label, error) {
labels := make([]*Label, 0, 10)
sess := x.Where("repo_id = ?", repoID)
switch sortType {
case "reversealphabetically":
sess.Desc("name")
case "leastissues":
sess.Asc("num_issues")
case "mostissues":
sess.Desc("num_issues")
default:
sess.Asc("name")
}
return labels, sess.Find(&labels)
}
func getLabelsByIssueID(e Engine, issueID int64) ([]*Label, error) {
var labels []*Label
return labels, e.Where("issue_label.issue_id = ?", issueID).
Join("LEFT", "issue_label", "issue_label.label_id = label.id").
Asc("label.name").
Find(&labels)
}
// GetLabelsByIssueID returns all labels that belong to given issue by ID.
func GetLabelsByIssueID(issueID int64) ([]*Label, error) {
return getLabelsByIssueID(x, issueID)
}
func updateLabel(e Engine, l *Label) error {
_, err := e.ID(l.ID).AllCols().Update(l)
return err
}
// UpdateLabel updates label information.
func UpdateLabel(l *Label) error {
return updateLabel(x, l)
}
// DeleteLabel delete a label of given repository.
func DeleteLabel(repoID, labelID int64) error {
_, err := GetLabelInRepoByID(repoID, labelID)
if err != nil {
if IsErrLabelNotExist(err) {
return nil
}
return err
}
sess := x.NewSession()
defer sess.Close()
if err = sess.Begin(); err != nil {
return err
}
if _, err = sess.ID(labelID).Delete(new(Label)); err != nil {
return err
} else if _, err = sess.
Where("label_id = ?", labelID).
Delete(new(IssueLabel)); err != nil {
return err
}
// Clear label id in comment table
if _, err = sess.Where("label_id = ?", labelID).Cols("label_id").Update(&Comment{}); err != nil {
return err
}
return sess.Commit()
}

Or if it is preferred to just have some code duplication and copy the existing functions and replacing repoID with orgID (it isn't too much here).

Duplicating them doesn't feel right, but hesitant to touch things that are working otherwise and just wanted some feedback as to which is preferred. The current API would remain the same for repo labels regardless so it would just be a refractor and not breaking change.

@lunny lunny added the type/enhancement An improvement of existing functionality label Jul 10, 2019
@lunny
Copy link
Member

lunny commented Jul 10, 2019

How about a label template cross site or in organization?

@mrsdizzie
Copy link
Member Author

@lunny organizations could use a template same as a repo does now with this change. It would be just like repo labels now with the same interface except they would apply for the entire organization (and repos would still have separate labels too). Just like organization webhooks, but for labels.

So if an organization go-gitea made a label that was kind/bug then every repo for that organization could use that label and if you did a search for that label_id you would see all bug issues for tea,gitea,go-sdk,etc... and you would not have to make and search separate kind/bug label for each repo. This is currently very difficult for people that have many repos and just want to have a normal bugand feature tag for all of them AND to see all issues at once. Repo label search would still work as normal since it would limit issues to that repo_id.

Quick/Bad mockup like this:

Screen Shot 2019-07-09 at 11 54 32 PM

So you could easily see issues in different repos with the same label.

@lunny
Copy link
Member

lunny commented Jul 10, 2019

When a repo has both self labels and organization labels, how to list them on issues labeling? Especially there are same labels.

@mrsdizzie
Copy link
Member Author

@lunny they could have both. Even now a repo lets you create two labels with the same name and information:

Screen Shot 2019-07-10 at 1 46 57 AM

So it would probably be up to user not to put two labels with the same name on an issue (there shouldn't be any reason to really -- if you want to see only bugs for one repo the issue search will allow that even with an organization label). I suspect most people will not want to create labels with the same name for repo and org.

I also think on some views there can be a visual difference for organization labels to make it more clear, like this:

Screen Shot 2019-07-10 at 1 40 29 AM

Users can select different colors too if they want or other ways to identify them. They would also be shown under a separate "organization" divider on the label assign list in issues so you can't easily pick the wrong one. There can probably be other little interface improvements too like adding an alt tag that says if a label belongs to an organization or repo.

@stale
Copy link

stale bot commented Sep 8, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Sep 8, 2019
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Sep 8, 2019
@stale stale bot removed the issue/stale label Sep 8, 2019
mrsdizzie added a commit to mrsdizzie/gitea that referenced this issue Mar 24, 2020
Implement organization wide labels similar to organization wide
webhooks. This lets you create individual labels for organizations that can be used
for all repos under that organization (so being able to reuse the same
label across multiple repos).

This makes it possible for small organizations with many repos to use
labels effectively.

Fixes go-gitea#7406
guillep2k added a commit that referenced this issue Apr 1, 2020
* Add organization wide labels

Implement organization wide labels similar to organization wide
webhooks. This lets you create individual labels for organizations that can be used
for all repos under that organization (so being able to reuse the same
label across multiple repos).

This makes it possible for small organizations with many repos to use
labels effectively.

Fixes #7406

* Add migration

* remove comments

* fix tests

* Update options/locale/locale_en-US.ini

Removed unused translation string

* show org labels in issue search label filter

* Use more clear var name

* rename migration after merge from master

* comment typo

* update migration again after rebase with master

* check for orgID <=0 per guillep2k review

* fmt

* Apply suggestions from code review

Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com>

* remove unused code

* Make sure RepoID is 0 when searching orgID per code review

* more changes/code review requests

* More descriptive translation var per code review

* func description/delete comment when issue label deleted instead of hiding it

* remove comment

* only use issues in that repo when calculating number of open issues for org label on repo label page

* Add integration test for IssuesSearch API with labels

* remove unused function

* Update models/issue_label.go

Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com>

* Use subquery in GetLabelIDsInReposByNames

* Fix tests to use correct orgID

* fix more tests

* IssuesSearch api now uses new BuildLabelNamesIssueIDsCondition. Add a few more tests as well

* update comment for clarity

* Revert previous code change now that we can use the new BuildLabelNamesIssueIDsCondition

* Don't sort repos by date in IssuesSearch API

After much debugging I've found a strange issue where in some cases MySQL will return a different result than other enigines if a query is sorted by a null collumn. For example with our integration test data where we don't set updated_unix in repository fixtures:

SELECT `id`, `owner_id`, `owner_name`, `lower_name`, `name`, `description`, `website`, `original_service_type`, `original_url`, `default_branch`, `num_watches`, `num_stars`, `num_forks`, `num_issues`, `num_closed_issues`, `num_pulls`, `num_closed_pulls`, `num_milestones`, `num_closed_milestones`, `is_private`, `is_empty`, `is_archived`, `is_mirror`, `status`, `is_fork`, `fork_id`, `is_template`, `template_id`, `size`, `is_fsck_enabled`, `close_issues_via_commit_in_any_branch`, `topics`, `avatar`, `created_unix`, `updated_unix` FROM `repository` ORDER BY updated_unix DESC LIMIT 15 OFFSET 45

Returns different results for MySQL than other engines. However, the similar query:

SELECT `id`, `owner_id`, `owner_name`, `lower_name`, `name`, `description`, `website`, `original_service_type`, `original_url`, `default_branch`, `num_watches`, `num_stars`, `num_forks`, `num_issues`, `num_closed_issues`, `num_pulls`, `num_closed_pulls`, `num_milestones`, `num_closed_milestones`, `is_private`, `is_empty`, `is_archived`, `is_mirror`, `status`, `is_fork`, `fork_id`, `is_template`, `template_id`, `size`, `is_fsck_enabled`, `close_issues_via_commit_in_any_branch`, `topics`, `avatar`, `created_unix`, `updated_unix` FROM `repository` ORDER BY updated_unix DESC LIMIT 15 OFFSET 30

Returns the same results.

This causes integration tests to fail on MySQL in certain cases but would never show up in a real installation. Since this API call always returns issues based on the optionally provided repo_priority_id or the issueID itself, there is no change to results by changing the repo sorting method used to get ids earlier in the function.

* linter is back!

* code review

* remove now unused option

* Fix newline at end of files

* more unused code

* update to master

* check for matching ids before query

* Update models/issue_label.go

Co-Authored-By: 6543 <6543@obermui.de>

* Update models/issue_label.go

* update comments

* Update routers/org/setting.go

Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>
Co-authored-by: 6543 <6543@obermui.de>
@lunny lunny added this to the 1.12.0 milestone Apr 1, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants