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

Projects enhancements #12506

Closed
wants to merge 4 commits into from
Closed

Conversation

patcito
Copy link
Contributor

@patcito patcito commented Aug 17, 2020

This branch builds on @adelowo project work in #8346. It implements:

  • order columns and issues and remember order after refresh
  • make it possible to see issue details in sidebar when clicking on it without leaving project page
  • make it possible to add new issues to project without leaving project page.

Here is the youtube demo to get an idea

Let me know what you think.

@patcito patcito mentioned this pull request Aug 17, 2020
13 tasks
Copy link
Member

@adelowo adelowo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but can we have migrations for ProjectBoard and ProjectIssues

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 17, 2020
@lunny lunny added the type/enhancement An improvement of existing functionality label Aug 17, 2020
@lunny lunny added this to the 1.13.0 milestone Aug 17, 2020
models/project_issue.go Outdated Show resolved Hide resolved
@patcito
Copy link
Contributor Author

patcito commented Aug 17, 2020

Looks good but can we have migrations for ProjectBoard and ProjectIssues

Thanks for the feedback. I've only added the Priority field which will be set to 0 automatically on first run, I think a migration would not be needed to upgrade.

models/project_board.go Outdated Show resolved Hide resolved
@6543
Copy link
Member

6543 commented Aug 17, 2020

@patcito can you make generate-swagger and commit the result?

@6543
Copy link
Member

6543 commented Aug 17, 2020

will close #11161

models/issue.go Outdated Show resolved Hide resolved
models/issue.go Outdated Show resolved Hide resolved
models/issue.go Outdated Show resolved Hide resolved
models/project.go Outdated Show resolved Hide resolved
models/project.go Outdated Show resolved Hide resolved
models/project.go Show resolved Hide resolved
routers/api/v1/repo/issue.go Outdated Show resolved Hide resolved
modules/auth/repo_form.go Outdated Show resolved Hide resolved
templates/repo/projects/view.tmpl Outdated Show resolved Hide resolved
templates/repo/projects/view.tmpl Outdated Show resolved Hide resolved
templates/repo/projects/view.tmpl Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Aug 17, 2020

Codecov Report

Merging #12506 into master will increase coverage by 0.02%.
The diff coverage is 28.27%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12506      +/-   ##
==========================================
+ Coverage   43.23%   43.26%   +0.02%     
==========================================
  Files         650      647       -3     
  Lines       72041    71701     -340     
==========================================
- Hits        31150    31019     -131     
+ Misses      35844    35660     -184     
+ Partials     5047     5022      -25     
Impacted Files Coverage Δ
models/issue.go 56.11% <0.00%> (-0.35%) ⬇️
models/migrations/migrations.go 4.66% <ø> (+2.20%) ⬆️
models/migrations/v148.go 0.00% <0.00%> (ø)
models/project_issue.go 20.74% <0.00%> (-6.45%) ⬇️
modules/auth/repo_form.go 40.86% <0.00%> (-1.48%) ⬇️
routers/repo/projects.go 12.90% <8.00%> (-0.29%) ⬇️
routers/repo/issue.go 37.48% <33.33%> (+0.36%) ⬆️
models/project_board.go 42.97% <55.88%> (-3.63%) ⬇️
models/project.go 47.40% <58.33%> (+0.92%) ⬆️
routers/api/v1/repo/issue.go 53.57% <100.00%> (+0.30%) ⬆️
... and 101 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0db9043...31ea178. Read the comment docs.

web_src/js/index.js Outdated Show resolved Hide resolved
@patcito
Copy link
Contributor Author

patcito commented Aug 17, 2020

I've pushed changes for all reviews, let me know what you think.

(except for this one #12506 (comment))

@almereyda
Copy link

I'm having a small suggestion, that would supposedly fit well into this PR, if it doesn't appear too new or foreign.

Can we also submit new projects with Ctrl + Enter instead of having to click on the button? This works nicely with issues, and would harmonise the UX and speed up usability a lot.

I don't know the codebase, but have checked out Gitea for the first time after Kanban landed, and would love to see it grow into something beautiful. Kudos to anyone involved here.

@silverwind
Copy link
Member

silverwind commented Nov 7, 2020

I think I've mentioned it before, but I'd like to see all those HTML ids changed to classes and the JS work with one or more elements of that class where it makes sense. This will the code easier to extend later.

I think HTML ids as an anti-feature, the only place where they are tolerable is as linkable anchors. Classes should be preferred because you never run into undefined behaviour when two instances of one thing are rendered.

@patcito
Copy link
Contributor Author

patcito commented Nov 7, 2020

It's a nightmare to constantly maintain this branch and too much work I'm not being paid for. I will keep maintaining it a bit more and then I'll stop. I understand it's your right not to merge it but it's also my right to abandon it. I'm also thinking of doing my own fork that would be less demanding to contributors because this level of nitpicking here is simply unsustainable to me though of course you are totally free to do so, just don't count me in.

@6543
Copy link
Member

6543 commented Nov 7, 2020

@patcito sorry if one of us had anoyed you - For me to review this pull was just hard (because of forcepushes ...) - And with the limited time I have I could not do good suggestions + realy take a look at the huge diff.

what I would do:
I think to get things in, you could start to pick single impruvement's out of this pull and create a new pull for each (I woud to it one by one ... so you only have to look at one at a time)

I'm also thinking of doing my own fork that would be less demanding ...

this is always your right to do so - the biggest downside on this, codebase will diverge when time passes and you have to resolve conflicts, each time you like to upgrade ...

@6543
Copy link
Member

6543 commented Nov 7, 2020

@almereyda just create an new issue specific for that enhancement :)

@patcito
Copy link
Contributor Author

patcito commented Nov 7, 2020

@patcito sorry if one of us had anoyed you - For me to review this pull was just hard (because of forcepushes ...) - And with the limited time I have I could not do good suggestions + realy take a look at the huge diff.

@6543 I'm not annoyed but I just don't have time to update this diff every week unfortunately.

what I would do:
I think to get things in, you could start to pick single impruvement's out of this pull and create a new pull for each (I woud to it one by one ... so you only have to look at one at a time)

You could cherry pick the commits that looks ok to you. In fact I'm working on allowing cherry picking commits from pull requests, it's in a private branch but I'm afraid it will take months to be accepted too...

I'm also thinking of doing my own fork that would be less demanding ...

this is always your right to do so - the biggest downside on this, codebase will diverge when time passes and you have to resolve conflicts, each time you like to upgrade ...

Yeah, that's why I'd rather have it merged than maintain a fork but I don't have time for neither unfortunately.

patcito added a commit to patcito/gitea that referenced this pull request Nov 10, 2020
as reported in https://drone.gitea.io/go-gitea/gitea/28939/1/4

use log.Error instead of log.Info

fix comments

fix eslint

fix goimport order

add migrations for project priorities

fix _repository.less lint errors

update swagger

fixes according to @6543 suggestions

s/NotInProjectID/ExcludeProjectID/g

update boards priority in a single transaction as per @zeripath

Co-authored-by: zeripath <[email protected]>

update swagger

Clean up the indenting

Co-authored-by: zeripath <[email protected]>

use tabs

Co-authored-by: zeripath <[email protected]>

use tabs

add loadRepository as per @zeripath suggestion

move projects css to features/projects.less
as per @silverwind request
go-gitea#12506 (review)

use transaction to update projects issues priorities
as per @zeripath suggestion
go-gitea#12506 (comment)

make import fit on one line

make use of loadRepository

indent html properly
as per suggestion by @silverwind
go-gitea#12506 (comment)

remove non-working code as per @zeripath suggestion
go-gitea#12506 (comment)

remove non-used code
as per @zeripath suggestion
go-gitea#12506 (comment)

fix syntax, remove content-type on request
go-gitea#12506 (comment)

use closest instead of parent
as per @silverwind suggestion
go-gitea#12506 (review)

use for instead of forEach because @silverwind

use for instead of forEach because @silverwind

go-gitea#12506 (review)

use // falls through as per @silverwind

go-gitea#12506 (comment)

listen to body keyup only on project page

go-gitea#12506 (review)

Update models/project_board.go

Co-authored-by: Lauris BH <[email protected]>

move all issue related code to its own util file
and re-use those functions in index.js and projects.js
as per @silverwind and @zeripath

Update models/project_issue.go

Co-authored-by: 6543 <[email protected]>

fix linting

Update web_src/js/features/issuesutil.js

Co-authored-by: silverwind <[email protected]>

Update web_src/js/features/issuesutil.js

Co-authored-by: silverwind <[email protected]>

Update web_src/js/features/issuesutil.js

Co-authored-by: silverwind <[email protected]>

Update web_src/js/features/projects.js

Co-authored-by: silverwind <[email protected]>

Update web_src/js/features/projects.js

Co-authored-by: silverwind <[email protected]>

Update models/project_issue.go

Update models/issue.go

Co-authored-by: zeripath <[email protected]>

Update web_src/js/features/issuesutil.js

Co-authored-by: silverwind <[email protected]>

various js fix per @silverwind suggestions

various fixes according to @silverwind and @zeripath

use await for fetch when possible

specify table name as per @lafriks suggestion

specify table name as per @lafriks suggestion in more queries

go-gitea#12506 (review)

go-gitea#12506 (comment)

Update templates/repo/projects/view.tmpl

Co-authored-by: silverwind <[email protected]>

Update templates/repo/projects/view.tmpl

Co-authored-by: silverwind <[email protected]>

Update web_src/js/features/projects.js

Co-authored-by: silverwind <[email protected]>

fix project_issue table name in query
make another fetch async as per @silverwind

Update models/project.go

Co-authored-by: Lauris BH <[email protected]>

Update models/project.go

Co-authored-by: Lauris BH <[email protected]>

more and rename UpdateBoards functions as per @6543

Update models/project.go

Co-authored-by: Lauris BH <[email protected]>

Update models/project.go

Co-authored-by: Lauris BH <[email protected]>

add / to route

fixup! fix backend lint as reported in https://drone.gitea.io/go-gitea/gitea/28939/1/4

Update models/project.go

Co-authored-by: zeripath <[email protected]>

Update routers/repo/issue.go

Co-authored-by: zeripath <[email protected]>

Update web_src/less/features/projects.less

Co-authored-by: silverwind <[email protected]>

fix uneeded column in query and remove css class
as suggested by @silverwind

Update models/project.go

Co-authored-by: Lauris BH <[email protected]>

add X to close sidebar and other css suggestions by @silverwind

add emoji to project issue titles

use issue.* instead as suggested by @zeripath
still waiting for xorm v1.0.5 to update go.mod

fix project_issue query for now

fix card details css
@mooror
Copy link

mooror commented Nov 11, 2020

For what it's worth @patcito, I am very grateful for all the time and effort you've put into this feature set.

I realize that probably doesn't mean much coming from a random guy who happens to be following along with the PR, but I thought I would say it anyway.

@6543
Copy link
Member

6543 commented Nov 14, 2020

had a look at it and tryed to split priority away: summary: frontend looks good so far but backend need a redo

as reported in https://drone.gitea.io/go-gitea/gitea/28939/1/4

use log.Error instead of log.Info

fix comments

fix eslint

fix goimport order

add migrations for project priorities

fix _repository.less lint errors

update swagger

fixes according to @6543 suggestions

s/NotInProjectID/ExcludeProjectID/g

update boards priority in a single transaction as per @zeripath

Co-authored-by: zeripath <[email protected]>

update swagger

Clean up the indenting

Co-authored-by: zeripath <[email protected]>

use tabs

Co-authored-by: zeripath <[email protected]>

use tabs

add loadRepository as per @zeripath suggestion

move projects css to features/projects.less
as per @silverwind request
go-gitea#12506 (review)

use transaction to update projects issues priorities
as per @zeripath suggestion
go-gitea#12506 (comment)

make import fit on one line

make use of loadRepository

indent html properly
as per suggestion by @silverwind
go-gitea#12506 (comment)

remove non-working code as per @zeripath suggestion
go-gitea#12506 (comment)

remove non-used code
as per @zeripath suggestion
go-gitea#12506 (comment)

fix syntax, remove content-type on request
go-gitea#12506 (comment)

use closest instead of parent
as per @silverwind suggestion
go-gitea#12506 (review)

use for instead of forEach because @silverwind

use for instead of forEach because @silverwind

go-gitea#12506 (review)

use // falls through as per @silverwind

go-gitea#12506 (comment)

listen to body keyup only on project page

go-gitea#12506 (review)

Update models/project_board.go

Co-authored-by: Lauris BH <[email protected]>

move all issue related code to its own util file
and re-use those functions in index.js and projects.js
as per @silverwind and @zeripath

Update models/project_issue.go

Co-authored-by: 6543 <[email protected]>

fix linting

Update web_src/js/features/issuesutil.js

Co-authored-by: silverwind <[email protected]>

Update web_src/js/features/issuesutil.js

Co-authored-by: silverwind <[email protected]>

Update web_src/js/features/issuesutil.js

Co-authored-by: silverwind <[email protected]>

Update web_src/js/features/projects.js

Co-authored-by: silverwind <[email protected]>

Update web_src/js/features/projects.js

Co-authored-by: silverwind <[email protected]>

Update models/project_issue.go

Update models/issue.go

Co-authored-by: zeripath <[email protected]>

Update web_src/js/features/issuesutil.js

Co-authored-by: silverwind <[email protected]>

various js fix per @silverwind suggestions

various fixes according to @silverwind and @zeripath

use await for fetch when possible

specify table name as per @lafriks suggestion

specify table name as per @lafriks suggestion in more queries

go-gitea#12506 (review)

go-gitea#12506 (comment)

Update templates/repo/projects/view.tmpl

Co-authored-by: silverwind <[email protected]>

Update templates/repo/projects/view.tmpl

Co-authored-by: silverwind <[email protected]>

Update web_src/js/features/projects.js

Co-authored-by: silverwind <[email protected]>

fix project_issue table name in query
make another fetch async as per @silverwind

Update models/project.go

Co-authored-by: Lauris BH <[email protected]>

Update models/project.go

Co-authored-by: Lauris BH <[email protected]>

more and rename UpdateBoards functions as per @6543

Update models/project.go

Co-authored-by: Lauris BH <[email protected]>

Update models/project.go

Co-authored-by: Lauris BH <[email protected]>

add / to route

fixup! fix backend lint as reported in https://drone.gitea.io/go-gitea/gitea/28939/1/4

Update models/project.go

Co-authored-by: zeripath <[email protected]>

Update routers/repo/issue.go

Co-authored-by: zeripath <[email protected]>

Update web_src/less/features/projects.less

Co-authored-by: silverwind <[email protected]>

fix uneeded column in query and remove css class
as suggested by @silverwind

Update models/project.go

Co-authored-by: Lauris BH <[email protected]>

add X to close sidebar and other css suggestions by @silverwind

add emoji to project issue titles

use issue.* instead as suggested by @zeripath
still waiting for xorm v1.0.5 to update go.mod

fix project_issue query for now

fix card details css
@patcito
Copy link
Contributor Author

patcito commented Nov 16, 2020

@6543 ok I'm going to close the issue because I can't commit to redo the backend. For those who want this feature I'll keep my fork up to date here https://github.com/patcito/gitea/tree/projects-enhancements

@patcito patcito closed this Nov 16, 2020
@lunny
Copy link
Member

lunny commented Nov 16, 2020

The PR is valuable and it should be splatted as several PRs to make the review easier.

@go-gitea go-gitea locked and limited conversation to collaborators Jan 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.