[need-help] Add pinned mod for repo #12498

a1012112796 · 2020-08-16T10:43:34Z

similar with github, use card ui to show pinned repos.

example view:

codecov-commenter · 2020-08-16T11:28:03Z

Codecov Report

Merging #12498 into master will decrease coverage by 0.02%.
The diff coverage is 36.08%.

@@            Coverage Diff             @@
##           master   #12498      +/-   ##
==========================================
- Coverage   43.23%   43.21%   -0.03%     
==========================================
  Files         650      652       +2     
  Lines       72041    72234     +193     
==========================================
+ Hits        31150    31217      +67     
- Misses      35844    35959     +115     
- Partials     5047     5058      +11

Impacted Files	Coverage Δ
models/migrations/migrations.go	`2.46% <ø> (ø)`
models/migrations/v150.go	`0.00% <0.00%> (ø)`
modules/auth/user_form.go	`39.65% <ø> (ø)`
modules/migrations/gitlab.go	`1.04% <0.00%> (-0.03%)`	⬇️
routers/org/setting.go	`0.00% <0.00%> (ø)`
routers/user/setting/profile.go	`30.40% <0.00%> (-11.20%)`	⬇️
routers/org/home.go	`55.00% <25.00%> (-10.91%)`	⬇️
models/error.go	`34.89% <40.00%> (-0.43%)`	⬇️
routers/user/profile.go	`51.75% <71.87%> (+3.28%)`	⬆️
models/user_pinnedrepo.go	`83.33% <83.33%> (ø)`
... and 20 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 7354efe...56b6f99. Read the comment docs.

lunny · 2020-08-16T15:38:46Z

I think the UI should be adjusted

lafriks · 2020-08-16T19:47:16Z

We should probably limit number of allowed pinned repositories per user/org

mrsdizzie · 2020-08-16T20:36:41Z

Cool : )

I think this should be a user (and org) setting and not repo. Github lets you pin repos you don't own but contribute to. Then user could also pin an org repo on their personal page too.

So a user would store a list of repos they have personally pinned instead of a repo being marked as pinned

a1012112796 · 2020-08-17T00:40:30Z

Cool : )

I think this should be a user (and org) setting and not repo. Github lets you pin repos you don't own but contribute to. Then user could also pin an org repo on their personal page too.

So a user would store a list of repos they have personally pinned instead of a repo being marked as pinned

Your suggestion is reasonable, but unfortunately we do not have a mechanism to find out whether a user has contribution in another repository, so we are not able to achieve the same effect with Github now, but I will adjust it to the user's property and wait for the introduction of the contribution statistics mechanism.

gsantner · 2020-08-17T11:43:09Z

My thought: Why not allow to pin any repository? Independent to whether or not you developed on it, or own it.

Just an example: Gitea org or a gitea developer could Pin gitnex android client, even though it's not under it's org, and the org/user didn't contribute to the project. To highlight a project is the main purpose after all, right?

mrsdizzie · 2020-08-17T13:56:17Z

@gsantner The main purpose for the page is to show an overview of repos and contributions made by a user and not to highlight just any project. Pinning just allows you to control what shows at the top when you have a large list that would move off screen. So if I would rather people see my Gitea contribution first instead of some test repos I made.

See Github pinning which this feature says to be based off of

gsantner · 2020-08-17T14:03:46Z

So if I would rather people see my Gitea contribution first instead of some test repos I made.

And I would say: So I would rather people see my highlight Gitea repos first instead of some test repos I made.

See Github pinning which this feature says to be based off of

Well, just because one platform has it that way, not means every others must copy-cat it in the same way.

mrsdizzie · 2020-08-17T14:33:05Z

Changing/breaking the behavior of the repository page which exists now with the intentional purpose of only showing a users repositories would be outside the scope of this PR.

There is already the stars page for listing unrelated repos that you like/want to highlight. Perhaps in the future a more general overview type page that has a favorites section as well with some pinned starred repos.

similar with github, use card ui to show pinned repos. Signed-off-by: a1012112796 <[email protected]>

silverwind · 2020-08-24T18:52:44Z

UI looks weird to me, especially the pin icon. Maybe make it a bit closer to what GH has?

a1012112796 · 2020-08-25T12:44:15Z

UI looks weird to me, especially the pin icon. Maybe make it a bit closer to what GH has?

Thounds great, but it means we should load all repo data to show config modal ui like gh which will break the original intention for split pages. Maybe the best way is to load data when press the "config button". But it's hard work for me(Sadly I have little knowledge about ui), and now maybe only heatmap uses this way in gitea. Or maybe we can add an individual page for configuring "pinnde repos". I wonder...

lunny · 2020-08-25T14:25:08Z

A close button or operation icons on the right top is better than the current design.

models/user_pinnedrepo.go

* remove unnecessary INDEX * fix transfer

silverwind · 2020-08-26T06:37:55Z

I can probably do a few style tweaks once the basic functionality is there. Are you planning to add a close button?

Drag-and-Drop might also be nice but would probably require a JS library.

a1012112796 · 2020-08-26T11:24:33Z

Are you planning to add a close button?

I'm sorry, But I can't understand the meanning of close button, Can you give me more detial? Thanks.

silverwind · 2020-08-26T11:37:53Z

A button that removes the pinned entry basically.

lunny · 2020-09-09T01:45:43Z

Could you resolve the conflicts?

lunny · 2020-09-10T04:48:53Z

@a1012112796 I think we need a new design. Current one needs some improvements.

silverwind · 2020-09-10T08:54:47Z

Yes, I think we should attempt to do it similar to GitHub, e.g. add primary language circle, remove the grey tag (it's already obvious that it's pinned based on the title above) and a small icon-button on the top-right to un-pin.

zeripath · 2020-09-10T09:06:41Z

options/locale/locale_en-US.ini

+pinned_repo_success = pinned repo success
+unpinned_repo_success = unpinned repo success


Suggested change

pinned_repo_success = pinned repo success

unpinned_repo_success = unpinned repo success

pinned_repo_success = Successfully pinned repo

unpinned_repo_success = Successfully unpinned repo

These also don't appear to be being used anywhere

zeripath · 2020-09-10T09:13:05Z

routers/org/home.go

+	for _, repo := range repos {
+		if has, err = org.IsPinnedRepoExist(repo.ID); err != nil {
+			ctx.ServerError("IsPinnedRepoExist", err)
+			return
+		}
+
+		if has {
+			pinnedRepos = append(pinnedRepos, repo)
+			repo.IsPinned = true
+		}
+	}


This is highly inefficient.

This code involves doing len(repos) individual queries against the db. We would be better off with a single query that returns all the pinned repos for an organization, putting them in a map and then checking each repo id in that set.

We know that the number of pinned repos is in general going to be smaller than the total number of repos in an organization. (certainly it's possible that a perverse organization might decide to pin all of their private repos and none of the public ones but that would be rare.)

Hmm, It's really a hard thing to balance memory usage and run-efficiency here. How about this? Thanks. 73ab596

zeripath · 2020-09-10T09:14:49Z

routers/user/profile.go

+		for _, repo := range repos {
+			if has, err = ctxUser.IsPinnedRepoExist(repo.ID); err != nil {
+				ctx.ServerError("IsPinnedRepoExist", err)
+				return
+			}
+
+			if has {
+				pinnedRepos = append(pinnedRepos, repo)
+				repo.IsPinned = true
+			}
+		}


Similar to the above comment. Not efficient.

zeripath · 2020-09-10T10:12:49Z

routers/user/profile.go

+		exitsPinnedRepoIDs := make([]int64, 0, len(pinnedRepos))
+		for _, repo := range pinnedRepos {
+			exitsPinnedRepoIDs = append(exitsPinnedRepoIDs, repo.ID)
+		}
+
+		var pinnedRepos2 []*models.Repository
+
+		pinnedRepos2, err = ctxUser.GetPinnedRepos(ctx.User, exitsPinnedRepoIDs, true)
+		if err != nil {
+			ctx.ServerError("GetPinnedRepos(true)", err)
+			return
+		}
+
+		if len(pinnedRepos2) > 0 {
+			for _, repo := range pinnedRepos2 {
+				repo.IsPinned = true
+			}
+			pinnedRepos = append(pinnedRepos, pinnedRepos2...)
+		}


Suggested change

exitsPinnedRepoIDs := make([]int64, 0, len(pinnedRepos))

for _, repo := range pinnedRepos {

exitsPinnedRepoIDs = append(exitsPinnedRepoIDs, repo.ID)

}

var pinnedRepos2 []*models.Repository

pinnedRepos2, err = ctxUser.GetPinnedRepos(ctx.User, exitsPinnedRepoIDs, true)

if err != nil {

ctx.ServerError("GetPinnedRepos(true)", err)

return

}

if len(pinnedRepos2) > 0 {

for _, repo := range pinnedRepos2 {

repo.IsPinned = true

}

pinnedRepos = append(pinnedRepos, pinnedRepos2...)

}

// Now add the ctxUsers other pinned repos that we can see.

// - but we should exclude all of the currently obtained repos as we don't want to get duplicates

excludeRepoIDs := make([]int64, 0, len(pinnedRepos))

for _, repo := range pinnedRepos {

excludeRepoIDs = append(excludeRepoIDs, repo.ID)

}

additionalPinnedRepos, err := ctxUser.GetPinnedRepos(ctx.User, excludeRepoIDs, true)

if err != nil {

ctx.ServerError("GetPinnedRepos(true)", err)

return

}

if len(additionalPinnedRepos) > 0 {

for _, repo := range additionalPinnedRepos {

repo.IsPinned = true

}

pinnedRepos = append(pinnedRepos, additionalPinnedRepos...)

}

routers/user/setting/profile.go

zeripath · 2020-09-10T11:20:39Z

web_src/js/index.js

+  $('.pinned-repo-btn').on('click', function (e) {
+    const $this = $(this);
+    e.preventDefault();
+    $.post($this.data('url'), {
+      _csrf: csrf,
+      name: $this.data('name'),
+      status: $this.data('status'),
+      is_btn: true,
+    }).done(reload());


We could return the new state of the button at the end of this and just update the button.... It seems strange to go to the effort of using a javascript to replace a form when you're not going to prevent a reload.

Indeed, unless the changes after the POST are significant work (button update is not), just update the DOM in JS without a reload, it's a much better user experience.

Indeed, unless the changes after the POST are significant work (button update is not), just update the DOM in JS without a reload, it's a much better user experience.

But It will not only change the button, but also will change the pinned repos list ...

I guess it's fine but sub-optimal. I know that in other parts of the UI we sometimes respond with HTML in the POST which is then replaced in JS, maybe that is an option.

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

mrsdizzie · 2020-09-11T13:27:58Z

Having another look since changes have been made -- the very large "UnPinned" button doesn't look very good here and the entire top area looks pretty cluttered now with 2 search bars on top of each other.

Again I think the Github UI is much better/cleaner for this

lunny · 2020-09-18T05:03:09Z

@a1012112796 Let's move this to v1.14 so that you have enough time to improve the UI.

stale · 2020-11-24T03:22:24Z

This pull request 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 months. Thank you for your contributions.

6543 · 2020-11-24T18:21:04Z

@a1012112796 would be nice if you have time to resolve conflicts :)

codecov-io · 2020-11-26T08:55:04Z

Codecov Report

Merging #12498 (e1f22fe) into master (03fa2ec) will increase coverage by 0.00%.
The diff coverage is 38.04%.

@@           Coverage Diff            @@
##           master   #12498    +/-   ##
========================================
  Coverage   42.24%   42.24%            
========================================
  Files         697      699     +2     
  Lines       76630    76814   +184     
========================================
+ Hits        32370    32452    +82     
- Misses      38937    39035    +98     
- Partials     5323     5327     +4

Impacted Files	Coverage Δ
models/migrations/migrations.go	`2.44% <ø> (ø)`
models/migrations/v160.go	`0.00% <0.00%> (ø)`
modules/auth/user_form.go	`39.65% <ø> (ø)`
routers/org/setting.go	`0.00% <0.00%> (ø)`
routers/user/setting/profile.go	`31.50% <0.00%> (-8.38%)`	⬇️
routers/org/home.go	`55.20% <25.00%> (-10.40%)`	⬇️
models/error.go	`36.80% <40.00%> (+0.05%)`	⬆️
routers/user/profile.go	`51.29% <71.87%> (+3.29%)`	⬆️
models/user_pinnedrepo.go	`83.33% <83.33%> (ø)`
models/models.go	`56.15% <100.00%> (+0.21%)`	⬆️
... and 18 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 03fa2ec...e1f22fe. Read the comment docs.

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 16, 2020

lunny added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Aug 16, 2020

lafriks added this to the 1.13.0 milestone Aug 16, 2020

a1012112796 marked this pull request as draft August 17, 2020 00:57

a1012112796 added 2 commits August 19, 2020 10:03

Add pinned mod for repo

5433a8c

similar with github, use card ui to show pinned repos. Signed-off-by: a1012112796 <[email protected]>

Merge branch 'master' into feature/Pinned_repo

156635a

a1012112796 force-pushed the feature/Pinned_repo branch from 7f94e75 to 156635a Compare August 19, 2020 02:07

a1012112796 marked this pull request as ready for review August 19, 2020 02:10

Merge branch 'master' into feature/Pinned_repo

3bd284a

lunny reviewed Aug 25, 2020

View reviewed changes

models/user_pinnedrepo.go Outdated Show resolved Hide resolved

lunny reviewed Aug 25, 2020

View reviewed changes

models/user_pinnedrepo.go Outdated Show resolved Hide resolved

a1012112796 added 3 commits August 26, 2020 10:14

Apply suggestions from code review, Thanks @lunny

184f140

* remove unnecessary INDEX * fix transfer

Merge branch 'master' into feature/Pinned_repo

c26b4f5

simplify ui

484ffc4

remove unused files

eb02288

ui

1356eb9

Merge branch 'master' into feature/Pinned_repo

6dcddf8

zeripath reviewed Sep 10, 2020

View reviewed changes

routers/user/setting/profile.go Outdated Show resolved Hide resolved

zeripath reviewed Sep 10, 2020

View reviewed changes

a1012112796 and others added 4 commits September 10, 2020 19:24

Apply suggestions from code review

0d400fd

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

try simplify logic

73ab596

Merge branch 'master' into feature/Pinned_repo

c88a9b8

fix delete repo

56b6f99

lunny modified the milestones: 1.13.0, 1.14.0 Sep 18, 2020

stale bot added the issue/stale label Nov 24, 2020

stale bot removed the issue/stale label Nov 24, 2020

Merge branch 'master' into feature/Pinned_repo

e1f22fe

a1012112796 changed the title ~~Add pinned mod for repo~~ [need-help] Add pinned mod for repo Nov 26, 2020

a1012112796 removed this from the 1.14.0 milestone Jan 27, 2021

a1012112796 closed this May 1, 2023

a1012112796 deleted the feature/Pinned_repo branch May 1, 2023 06:30

go-gitea locked as resolved and limited conversation to collaborators Aug 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[need-help] Add pinned mod for repo #12498

[need-help] Add pinned mod for repo #12498

a1012112796 commented Aug 16, 2020 •

edited

Loading

codecov-commenter commented Aug 16, 2020 •

edited

Loading

lunny commented Aug 16, 2020

lafriks commented Aug 16, 2020

mrsdizzie commented Aug 16, 2020

a1012112796 commented Aug 17, 2020

gsantner commented Aug 17, 2020 •

edited

Loading

mrsdizzie commented Aug 17, 2020

gsantner commented Aug 17, 2020 •

edited

Loading

mrsdizzie commented Aug 17, 2020

silverwind commented Aug 24, 2020

a1012112796 commented Aug 25, 2020

lunny commented Aug 25, 2020

silverwind commented Aug 26, 2020

a1012112796 commented Aug 26, 2020

silverwind commented Aug 26, 2020

lunny commented Sep 9, 2020

lunny commented Sep 10, 2020

silverwind commented Sep 10, 2020

zeripath Sep 10, 2020

zeripath Sep 10, 2020

zeripath Sep 10, 2020

a1012112796 Sep 10, 2020

zeripath Sep 10, 2020

zeripath Sep 10, 2020

zeripath Sep 10, 2020

silverwind Sep 10, 2020

a1012112796 Sep 10, 2020

silverwind Sep 16, 2020

mrsdizzie commented Sep 11, 2020

lunny commented Sep 18, 2020

stale bot commented Nov 24, 2020

6543 commented Nov 24, 2020

codecov-io commented Nov 26, 2020

		pinned_repo_success = pinned repo success
		unpinned_repo_success = unpinned repo success

[need-help] Add pinned mod for repo #12498

[need-help] Add pinned mod for repo #12498

Conversation

a1012112796 commented Aug 16, 2020 • edited Loading

codecov-commenter commented Aug 16, 2020 • edited Loading

Codecov Report

lunny commented Aug 16, 2020

lafriks commented Aug 16, 2020

mrsdizzie commented Aug 16, 2020

a1012112796 commented Aug 17, 2020

gsantner commented Aug 17, 2020 • edited Loading

mrsdizzie commented Aug 17, 2020

gsantner commented Aug 17, 2020 • edited Loading

mrsdizzie commented Aug 17, 2020

silverwind commented Aug 24, 2020

a1012112796 commented Aug 25, 2020

lunny commented Aug 25, 2020

silverwind commented Aug 26, 2020

a1012112796 commented Aug 26, 2020

silverwind commented Aug 26, 2020

lunny commented Sep 9, 2020

lunny commented Sep 10, 2020

silverwind commented Sep 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrsdizzie commented Sep 11, 2020

lunny commented Sep 18, 2020

stale bot commented Nov 24, 2020

6543 commented Nov 24, 2020

codecov-io commented Nov 26, 2020

Codecov Report

a1012112796 commented Aug 16, 2020 •

edited

Loading

codecov-commenter commented Aug 16, 2020 •

edited

Loading

gsantner commented Aug 17, 2020 •

edited

Loading

gsantner commented Aug 17, 2020 •

edited

Loading