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.

Sign up for GitHub

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

Jump to bottom

Improve and fix bugs in runner management page #24366

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

Improve and fix bugs in runner management page #24366

wants to merge 25 commits into from
+87 −18

Conversation

yp05327
Copy link
Contributor

@yp05327 yp05327 commented Apr 27, 2023 •
edited
Loading

Changes:

  • Add no permission to edit tooltip
    image
  • fix the logic in ActionRunner.Editable
    • Gitea admin user can edit runner in org/repo runner settings page
      image
    • Org owner can edit org/repo runner in org's repo runner settings page
      image
    • org admin can only edit org's repo runner
      image
  • add missed locale word: runners.task_list.no_tasks
  • fix overflow of Recent tasks on this runner This has been fixed.
    Before:
    image
    After:
    image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 27, 2023
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 27, 2023
@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 27, 2023
@yp05327 yp05327 changed the title [WIP] Add no permission to edit tooltip and fix the logic of ActionRunner.Editable Add no permission to edit tooltip and fix the logic of ActionRunner.Editable Apr 27, 2023
@yp05327 yp05327 changed the title Add no permission to edit tooltip and fix the logic of ActionRunner.Editable Add no permission notification and enable editing editable runners in each runner management page Apr 27, 2023
@yp05327 yp05327 changed the title Add no permission notification and enable editing editable runners in each runner management page Add no permission notification and enable editing all editable runners in each runner management page Apr 27, 2023
@yp05327 yp05327 added the topic/ui Change the appearance of the Gitea UI label Apr 27, 2023
@silverwind
Copy link
Member

image

Edit icon looks slighly misaligned with cross icon horizontally.

@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 May 1, 2023
@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 May 1, 2023
@yp05327 yp05327 changed the title Add no permission notification and enable editing all editable runners in each runner management page Improve and fix bugs in runner management page May 1, 2023
@yp05327
Copy link
Contributor Author

yp05327 commented May 1, 2023

image

Edit icon looks slighly misaligned with cross icon horizontally.

Done.

silverwind
silverwind reviewed May 1, 2023
View reviewed changes
web_src/css/runner.css Outdated Show resolved Hide resolved
wolfogre
wolfogre reviewed May 4, 2023
View reviewed changes
Copy link
Member

@wolfogre wolfogre left a comment

Choose a reason for hiding this comment

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

After this, it will be possible to edit a global/org runner on the settings page of a repo if the doer has permission. It doesn't look really good to me.

image

image

Just my opinion, I would say it could be better to redirect to org/admin settings page.

models/actions/runner.go Outdated Show resolved Hide resolved
@denyskon
Copy link
Member

@yp05327 Is this PR still relevant? If so, could you update it?

@denyskon denyskon added the issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail label Jan 21, 2024
@yp05327
Copy link
Contributor Author

yp05327 commented Jan 22, 2024

I'll try to update it.

@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 Jan 22, 2024
@yp05327
Copy link
Contributor Author

yp05327 commented Jan 22, 2024

Added individual runner support.

@yp05327
Copy link
Contributor Author

yp05327 commented Jan 22, 2024

Update is done.

@lunny lunny added this to the 1.22.0 milestone Jan 22, 2024
@lunny
Copy link
Member

lunny commented Jan 22, 2024

After this, it will be possible to edit a global/org runner on the settings page of a repo if the doer has permission. It doesn't look really good to me.

image image

Just my opinion, I would say it could be better to redirect to org/admin settings page.

It seems this hasn't been followed?

@yp05327
Copy link
Contributor Author

yp05327 commented Jan 23, 2024

This has been supported in 110b928

But I noticed a problem. If admin want to edit a individual runner from admin panel which doesn't belong to him, which URL should we use?
/user/settings/actions/runners/{id} seems strange as this runner doesn't belong to him.
But if we use /admin/actions/runners/{id}, then we have several entry points to this runner ( but for the admin user, there's only one )

@denyskon
Copy link
Member

Isn't the second option used currently? Looks correct to me, the admin should always use admin routes if possible

@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 Jan 26, 2024
@lunny lunny modified the milestones: 1.22.0, 1.23.0 Mar 29, 2024
@lunny lunny modified the milestones: 1.23.0, 1.24.0 Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wolfogre wolfogre wolfogre left review comments

@silverwind silverwind silverwind left review comments

Assignees
No one assigned
Labels
issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/translation size/L Denotes a PR that changes 100-499 lines, ignoring generated files. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Milestone
1.24.0
Development

Successfully merging this pull request may close these issues.
6 participants
@yp05327 @silverwind @denyskon @lunny @wolfogre @GiteaBot