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

[Feature] Private README.md for organization #32872

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

changchaishi
Copy link
Contributor

@changchaishi changchaishi commented Dec 17, 2024

Implemented #29503

Changes:

  1. Since individual users and organizations use the same .profile repository name for README.md, adding the private one, I rename the contexts with Public and Private inside them.
  2. I wanted to load the private profile the same as GitHub, which uses the view_as query parameter. Still, the existing query scheme is quite limited due to the integration with the search form and the repository paginator, so my workaround is to forcefully bring augmented query strings of member/public to the tmpl file.
  3. The drop-down is adopted from the search box's sort by component, we need to discuss the stylings.

Things lack and need further guidance:

  1. At the create repository page, the ideal user experience is when the repo name is entered .profile the make repository private checkbox should be disabled and stay unchecked, whereas .profile-private, the checkbox should be disabled and force checked.
  2. Follow 1., Tool tips should be added.
  3. The translation and CSS for the current drop box and view as public/member feature.

Screenshots
Demo:
drop_down

Need help:
auto_check_and_disable

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 17, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 17, 2024
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files labels Dec 17, 2024
templates/org/home.tmpl Outdated Show resolved Hide resolved
@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 Dec 23, 2024
@changchaishi changchaishi marked this pull request as draft December 23, 2024 06:02
@changchaishi
Copy link
Contributor Author

Hi, I listed the behaviors that this PR implemented as below:

  • hints to create .profile and .profile-private repos (if users do not follow the hint, profile may not show)
    private_hint

  • view_as drop down only shows when the user is member in organization and both public and member profiles exist
    only_shows_when_2_present

  • If there is no profile repo, not showing overview tab
    no_profiles_no_overview

  • When user is a member of org and logged in, and only 1 profile is present, show that profile
    public_only_show_public
    private_shows_private

  • When user is not logged in, or not a member, only public profile will show if it is present
    non_member_can_show

@changchaishi changchaishi marked this pull request as ready for review December 23, 2024 06:10
@wxiaoguang
Copy link
Contributor

Could you add some tests in tests/integration/org_*.go to verify the "view as public" and "view as member" outputs?

If I understand correctly: view as public: private repositories won't be listed.

@changchaishi
Copy link
Contributor Author

If I understand correctly: view as public: private repositories won't be listed.

In this design view as is only to select profiles. The permission system decides whether the doer can access the README.md. Nothing has changed.

So viewing as public does not mean that a member user will not see private repos listed if he/she already has permission.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Dec 27, 2024

If I understand correctly: view as public: private repositories won't be listed.

In this design view as is only to select profiles. The permission system decides whether the doer can access the README.md. Nothing has changed.

Maybe GitHub also changed their behavior: You are viewing the README and **pinned** repositories as a public user. vs You are viewing the README and repositories as a public user. I wouldn't say it is good but it doesn't block since GitHub does so.

While I still think it needs to add some tests in tests/integration/org_*.go to verify the "view as public" and "view as member" outputs, to cover the new code's behavior.

@changchaishi
Copy link
Contributor Author

While I still think it needs to add some tests in tests/integration/org_*.go to verify the "view as public" and "view as member" outputs, to cover the new code's behavior.

I'm working on it. Trying to get familiar with the integration tests.

@lunny lunny added this to the 1.24.0 milestone Dec 29, 2024
@@ -44,6 +44,8 @@
<div class="inline required field {{if .Err_RepoName}}error{{end}}">
<label for="repo_name">{{ctx.Locale.Tr "repo.repo_name"}}</label>
<input id="repo_name" name="repo_name" value="{{.repo_name}}" autofocus required maxlength="100">
<span id="repo_name_public_profile_hint" style="display:none" class="help">{{ctx.Locale.Tr "repo.repo_name_public_profile_hint"}}</span>
Copy link
Member

Choose a reason for hiding this comment

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

Use tw-hidden class rather than style="display:none".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/frontend modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/translation size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants