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

Fix broken following organization #29005

Merged
merged 9 commits into from
Feb 17, 2024
Merged

Fix broken following organization #29005

merged 9 commits into from
Feb 17, 2024

Conversation

yp05327
Copy link
Contributor

@yp05327 yp05327 commented Feb 1, 2024

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 1, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 1, 2024
@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 1, 2024
@GiteaBot GiteaBot 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 Feb 1, 2024
@yardenshoham
Copy link
Member

Sorry for breaking

@yp05327
Copy link
Contributor Author

yp05327 commented Feb 1, 2024

Sorry for breaking

Never mind.

@yp05327
Copy link
Contributor Author

yp05327 commented Feb 1, 2024

@yardenshoham
I can't understand why we not only refresh follow/unfollow button, but refresh the whole profile-avatar-card

I see, we need to refresh the following count in user profile avatar card, but we don't need to refresh it in organization's home page. So only refresh the follow button is enough. I will make a patch for this.

@yardenshoham
Copy link
Member

So you can remove hx-target when templating org

@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 1, 2024
@yp05327
Copy link
Contributor Author

yp05327 commented Feb 9, 2024

I think this should be in 1.22's milestone, as #28908 will be released in 1.22.

@yp05327 yp05327 added this to the 1.22.0 milestone Feb 9, 2024
ctx.HTML(http.StatusOK, tplFollowUnfollow)
return
}
log.Error("Failed to apply action %q: unsupport context user type: %s", ctx.FormString("action"), ctx.ContextUser.Type)
Copy link
Member

Choose a reason for hiding this comment

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

ctx.ContextUser.Type is not a %s.

Comment on lines +326 to +332
} else if ctx.ContextUser.IsOrganization() {
ctx.Data["IsFollowing"] = ctx.Doer != nil && user_model.IsFollowing(ctx, ctx.Doer.ID, ctx.ContextUser.ID)
ctx.HTML(http.StatusOK, tplFollowUnfollow)
return
}
log.Error("Failed to apply action %q: unsupport context user type: %s", ctx.FormString("action"), ctx.ContextUser.Type)
ctx.Error(http.StatusBadRequest, fmt.Sprintf("Action %q failed", ctx.FormString("action")))
Copy link
Member

Choose a reason for hiding this comment

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

Why doesn't the same action apply as for users?
Does the org not use HTMX on that page?
If no, then I'd suggest adding it there as well.
And in the other case, I don't see reasons to deviate from the user behavior…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

user profile has the count of followers, that should also be refreshed. But org home page doesn’t.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense, unfortunately.
Ultimately, I think the behavior should be unified, but okay, I can accept that for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This also confused me.
You can check the comment: #29005 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

I can take a stab at this after this PR is merged

Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

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

Sorry, forgot the approval yesterday

@GiteaBot GiteaBot 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 Feb 10, 2024
@yardenshoham yardenshoham added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 16, 2024
@lunny lunny merged commit 6822799 into go-gitea:main Feb 17, 2024
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 17, 2024
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this pull request Feb 19, 2024
Leftover from go-gitea/gitea#29005

# Before

![before](https://github.com/go-gitea/gitea/assets/20454870/24c74278-ccac-4dc6-bf26-713e90c07239)

# After

![after](https://github.com/go-gitea/gitea/assets/20454870/f91d503b-87d4-4c17-a56c-9c0a81fd9082)

---------

Signed-off-by: Yarden Shoham <git@yardenshoham.com>
(cherry picked from commit aa6f88638fb827d5c5ed7506e5fc06dad92beea7)
silverwind pushed a commit to silverwind/gitea that referenced this pull request Feb 20, 2024
- following organization is broken from go-gitea#28908
- add login check for the follow button in organization profile page
6543 pushed a commit to 6543-forks/gitea that referenced this pull request Feb 26, 2024
- following organization is broken from go-gitea#28908
- add login check for the follow button in organization profile page

(cherry picked from commit 6822799)
Copy link

Automatically locked because of our CONTRIBUTING guidelines

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 28, 2024
@yp05327 yp05327 deleted the fix-org-follow branch March 13, 2024 02:18
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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants