Skip to content

Conversation

@ethantkoenig
Copy link
Member

Fix bug where num_following fields were not correctly updated on user delete.

Also add an integration test for deleting users.

@sapk
Copy link
Member

sapk commented May 12, 2017

Why rename models/consistency_test.go → models/consistency.go ?

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 12, 2017
@lunny lunny added this to the 1.2.0 milestone May 12, 2017
@lunny lunny added the type/bug label May 12, 2017
@ethantkoenig
Copy link
Member Author

@sapk So that it can be used in integrations/; you can't import files ending in _test.go.

models/user.go Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this could be changed to []int64 array and select only FollowID values into this array. I don't know what is the limitation of count for In(field, array) but it could be optimized to update it in batches so that there would be less requests to database

Copy link
Member Author

Choose a reason for hiding this comment

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

@lafriks I have an updated version, but it's blocked on a fix to xorm (go-xorm/builder#4)

Copy link
Member

Choose a reason for hiding this comment

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

@ethantkoenig fix seems to be merged in xorm, you can now fetch dependency and it should be ok now

Copy link
Member Author

Choose a reason for hiding this comment

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

pushed

models/user.go Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Same here as with followees

@lunny
Copy link
Member

lunny commented May 20, 2017

LGTM

@tboerger tboerger 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 May 20, 2017
@lafriks
Copy link
Member

lafriks commented May 20, 2017

LGTM

@tboerger tboerger 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 May 20, 2017
@lunny lunny merged commit cf02cd7 into go-gitea:master May 20, 2017
@ethantkoenig ethantkoenig deleted the delete_user branch May 20, 2017 23:35
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants