Conversation
mieliespoor
left a comment
There was a problem hiding this comment.
on a high level, this looks acceptable. Will need to test it to be more confident.
models/project.go
Outdated
| @@ -0,0 +1,68 @@ | |||
|
|
|||
There was a problem hiding this comment.
can you remove this whitespace?
There was a problem hiding this comment.
I think these changes can be excluded from the PR. This is mainly local dev requirements, so not sure we should include it in the PR?
|
Can you change the title of the PR from Hello UI to something more relevant to the changes? |
Ohhk i will do that |
|
@mieliespoor what's the status? |
|
helo @mieliespoor @klesh somebody plz review this |
| // Load associated tags | ||
| db.Model(&project).Association("Tags").Find(&project.Tags) | ||
|
|
||
| c.JSON(200, project) |
There was a problem hiding this comment.
| c.JSON(200, project) | |
| c.JSON(http.StatusOK, project) |
Should this not also be http.StatusOK instead of the explicit 200? Makes for easier reading.
|
Added one more comment around readability and consistency. Looks good on a highlevel. Two things I would say that is missing on this PR:
Other than that, I would allow the code owners to also review this. |
| ) | ||
|
|
||
| // Project represents a DevLake project | ||
| type Project struct { |
There was a problem hiding this comment.
There is a Project model located in https://github.com/apache/incubator-devlake/blob/main/backend/core/models/project.go
Please following the existing folder convention.
- All backend code should be placed in the
backendfolder. - UI code should go to the
config-uifolder
|
This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs. |
|
@ArkVex are you still interested in this? |
|
This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs. |
|
Any chance of this being merged in to the project soon? @ArkVex I'd rather not maintain my own image for this feature if possible |
|
I think he lost interest. I will take a look at this and try make time to get this merged in via a new PR as I myself need this relatively soonish. |
|
|
||
| // Tag represents a tag that can be assigned to projects | ||
| type Tag struct { | ||
| common.DynamicMapBase |
There was a problem hiding this comment.
I cannot find the definition of DynamicMapBase in common package.
|
|
||
| // ProjectTag represents the many-to-many relationship between projects and tags | ||
| type ProjectTag struct { | ||
| ProjectId string `json:"projectId" gorm:"primaryKey;type:varchar(255)"` |
There was a problem hiding this comment.
Project in package common doesn't have ProjectId field, I think it should be ProjectName here.
| // ProjectTag represents the many-to-many relationship between projects and tags | ||
| type ProjectTag struct { | ||
| ProjectId string `json:"projectId" gorm:"primaryKey;type:varchar(255)"` | ||
| TagId string `json:"tagId" gorm:"primaryKey;type:varchar(255)"` |
There was a problem hiding this comment.
Since Tag.Name has an unique index, we can use TagName here.
| @@ -0,0 +1,52 @@ | |||
| package migrationscripts | |||
There was a problem hiding this comment.
pr-type/bug-fix,pr-type/feature-development, etc.Summary
This PR introduces a project tagging system, allowing users to assign and manage tags for better organization. It updates the database schema to store tags, enhances the API to support tag-based filtering, and improves the UI for easy tag management. This feature helps tech leads and engineering managers track projects efficiently and create dashboards based on shared tags.
Does this close any open issues?
Closes #8279
Screenshots
Include any relevant screenshots here.
Other Information
Any other information that is important to this PR.