Fix not removed watches on unallowed repositories #4201

daviian · 2018-06-09T21:55:49Z

Targets: #3343, #3782, #4149

What does it do:

removes watches if users gets removed from a team or a team is deleted as a whole.
also sets new team users to watch all team repositories.
remove watch if user gets removed from the collaborators of a repo

TODO:

create migration to remove watches that are hanging in the air
also remove issue_watches
also remove issue_watches in migration

codecov-io · 2018-06-09T22:03:17Z

Codecov Report

Merging #4201 into master will increase coverage by 0.01%.
The diff coverage is 29.41%.

@@            Coverage Diff             @@
##           master    #4201      +/-   ##
==========================================
+ Coverage   19.97%   19.98%   +0.01%     
==========================================
  Files         153      153              
  Lines       30530    30581      +51     
==========================================
+ Hits         6097     6112      +15     
- Misses      23519    23542      +23     
- Partials      914      927      +13

Impacted Files	Coverage Δ
models/repo_collaboration.go	`64.22% <0%> (-3.3%)`	⬇️
models/repo.go	`17.78% <0%> (-0.04%)`	⬇️
models/issue_watch.go	`76.47% <100%> (+5.73%)`	⬆️
models/org_team.go	`50.22% <15.62%> (-2.75%)`	⬇️

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 467ff4d...75f9efb. Read the comment docs.

daviian · 2018-06-09T23:11:55Z

@lafriks Please add status/wip as I've to add the migration to cleanup hanging watches

bkcsoft · 2018-06-15T08:18:22Z

@daviian I don't think that the mgiration is necessary for this fix. It would be nice to have, but not something that should hold back this PR.

daviian · 2018-06-15T08:23:32Z

@bkcsoft It's already implemented. I'm currently in the middle of testing the migration locally ;) If it's fine i'll push.

lunny · 2018-06-15T12:32:10Z

models/migrations/v67.go

+		return a.Mode, nil
+	}
+
+	sess := x.NewSession()


need

defer sess.Close() if err := sess.Begin(); err != nil { return err }

Oh yes. Totally forgot.

lunny · 2018-06-15T12:33:53Z

models/org_team.go

@@ -178,6 +178,11 @@ func (t *Team) removeRepository(e Engine, repo *Repository, recalculate bool) (e
 		if err = watchRepo(e, teamUser.UID, repo.ID, false); err != nil {
 			return err
 		}
+
+		// Remove all IssueWatches a user has subscribed to in the repositories
+		if err := removeIssueWatchersByRepoID(e, teamUser.ID, repo.ID); err != nil {


teamUser.UID

daviian · 2018-06-15T12:51:58Z

@lunny Added your suggestions

… a whole

…removed, a collaborator is removed

…statement

lunny · 2018-06-19T04:51:40Z

models/migrations/v67.go

+				}
+				if _, err := x.Get(repo); err != nil {
+					return err
+				}


You should repoCache[watch.RepoID] = repo

lunny · 2018-06-19T04:52:16Z

models/migrations/v67.go

+					}
+					if _, err := x.Get(repo); err != nil {
+						return err
+					}


The same as above

lunny · 2018-06-19T05:53:57Z

Please follow my comment, otherwise LGTM

techknowlogick · 2018-06-19T19:45:32Z

@lafriks the ticket this was attached to was marked as backport, but you removed the label from this PR, should it still be backported?

lunny · 2018-06-20T08:30:17Z

This will not be back port since it has a migration which will let user difficult upgrade from v1.4 to v1.5

daviian changed the title ~~Bugfix/watches~~ Fix not removed watches on unallowed repositories Jun 9, 2018

bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 9, 2018

lafriks approved these changes Jun 9, 2018

View reviewed changes

bkcsoft added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 9, 2018

lafriks added the type/bug label Jun 9, 2018

lafriks added this to the 1.5.0 milestone Jun 9, 2018

techknowlogick added the pr/wip This PR is not ready for review label Jun 10, 2018

daviian force-pushed the bugfix/watches branch from 9188eee to 563c066 Compare June 12, 2018 22:19

daviian force-pushed the bugfix/watches branch from 563c066 to 9b96458 Compare June 15, 2018 08:52

daviian removed the pr/wip This PR is not ready for review label Jun 15, 2018

lunny requested changes Jun 15, 2018

View reviewed changes

daviian added 10 commits June 15, 2018 16:59

remove watches if user gets removed from team, and team is deleted as…

4baa117

… a whole

let new team member automatically watch all repositories

4774973

remove watch if user gets removed from collaborators

a7a0b85

undo unneeded change

ffc60ff

remove stale watches

fd3cf7e

remove issue_watches when team member is removed, team repository is …

7d54006

…removed, a collaborator is removed

remove stale issue watches; fix missing userID in issue_watch delete …

fa758ae

…statement

add missing beginning and closing of session in migration

dcd1cba

change to correct user id

edb0169

remove debug log output from migration

39f51c5

daviian force-pushed the bugfix/watches branch from becb260 to 39f51c5 Compare June 15, 2018 14:59

lunny requested changes Jun 19, 2018

View reviewed changes

bkcsoft added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 19, 2018

daviian and others added 2 commits June 19, 2018 15:36

build up cache

4525560

Merge branch 'master' into bugfix/watches

ab794a3

lunny approved these changes Jun 19, 2018

View reviewed changes

Merge branch 'master' into bugfix/watches

75f9efb

techknowlogick merged commit a93f138 into go-gitea:master Jun 19, 2018

This was referenced Jun 19, 2018

Members removed from team/repository keep watches #3782

Closed

New user does not see changes on dashboard #3343

Closed

daviian deleted the bugfix/watches branch June 20, 2018 05:09

6543 mentioned this pull request Feb 23, 2020

Code Refactor of IssueWatch related things #10401

Merged

go-gitea locked and limited conversation to collaborators Nov 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix not removed watches on unallowed repositories #4201

Fix not removed watches on unallowed repositories #4201

daviian commented Jun 9, 2018 •

edited

codecov-io commented Jun 9, 2018 •

edited

daviian commented Jun 9, 2018

bkcsoft commented Jun 15, 2018

daviian commented Jun 15, 2018 •

edited

lunny Jun 15, 2018

daviian Jun 15, 2018

lunny Jun 15, 2018

daviian commented Jun 15, 2018

lunny Jun 19, 2018

lunny Jun 19, 2018

lunny commented Jun 19, 2018

techknowlogick commented Jun 19, 2018

lunny commented Jun 20, 2018

Fix not removed watches on unallowed repositories #4201

Fix not removed watches on unallowed repositories #4201

Conversation

daviian commented Jun 9, 2018 • edited

codecov-io commented Jun 9, 2018 • edited

Codecov Report

daviian commented Jun 9, 2018

bkcsoft commented Jun 15, 2018

daviian commented Jun 15, 2018 • edited

lunny Jun 15, 2018

Choose a reason for hiding this comment

daviian Jun 15, 2018

Choose a reason for hiding this comment

lunny Jun 15, 2018

Choose a reason for hiding this comment

daviian commented Jun 15, 2018

lunny Jun 19, 2018

Choose a reason for hiding this comment

lunny Jun 19, 2018

Choose a reason for hiding this comment

lunny commented Jun 19, 2018

techknowlogick commented Jun 19, 2018

lunny commented Jun 20, 2018

daviian commented Jun 9, 2018 •

edited

codecov-io commented Jun 9, 2018 •

edited

daviian commented Jun 15, 2018 •

edited