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

Step logs removing API and Button #3451

Merged
merged 24 commits into from
Apr 14, 2024

Conversation

zc-devs
Copy link
Contributor

@zc-devs zc-devs commented Feb 27, 2024

Closes #3444
closes #1272


Admin:
Screenshot 2024-02-27 1
Screenshot 2024-02-27 2
Screenshot 2024-02-27 3

User:
Screenshot 2024-02-27 4

@qwerty287 qwerty287 added the feature add new functionality label Feb 27, 2024
@6543

This comment was marked as outdated.

@woodpecker-bot
Copy link
Collaborator

woodpecker-bot commented Feb 28, 2024

Deployment of preview was torn down

@6543
Copy link
Member

6543 commented Feb 28, 2024

also as per CI :D -> go generate cmd/server/swagger.go and commit

@zc-devs
Copy link
Contributor Author

zc-devs commented Feb 28, 2024

also as per CI :D -> go generate cmd/server/swagger.go and commit

Unfortunately, nothing changed :(
go-generate-swagger.log

@zc-devs zc-devs mentioned this pull request Mar 1, 2024
@zc-devs
Copy link
Contributor Author

zc-devs commented Mar 1, 2024

usr before deleting (admin also have access to this button):

Screenshot 2024-03-01 211151

usr2 after deleteion, the same for guest:

Screenshot 2024-03-01 211525

Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

code looks good - did not test it jet

@qwerty287
Copy link
Contributor

Can you allow this also for repo push access? The endpoint to delete the whole pipeline's logs is also available with push access only.

@anbraten anbraten added the ui frontend related label Mar 5, 2024
@lafriks
Copy link
Contributor

lafriks commented Mar 20, 2024

Ideally would be if the delete button would be hidden under the sub menu under the button with horizontal ... icon or something like that (not requirement for this PR)

@xoxys xoxys mentioned this pull request Mar 21, 2024
@qwerty287 qwerty287 added this to the 2.5.0 milestone Mar 21, 2024
server/api/pipeline.go Outdated Show resolved Hide resolved
web/src/components/repo/pipeline/PipelineLog.vue Outdated Show resolved Hide resolved
Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

just for UX concerns, can we add a confirmation question popup in the WebUI ?

Copy link
Contributor

@qwerty287 qwerty287 left a comment

Choose a reason for hiding this comment

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

Tested, working

@qwerty287 qwerty287 requested a review from 6543 March 24, 2024 14:44
@qwerty287
Copy link
Contributor

qwerty287 commented Mar 28, 2024

@6543 you requested changes, but your concerns were addressed
@zc-devs Can you fix the linters?

Copy link

codecov bot commented Mar 28, 2024

Codecov Report

Attention: Patch coverage is 0% with 39 lines in your changes are missing coverage. Please review.

Project coverage is 35.92%. Comparing base (8e45ddd) to head (b9fc970).
Report is 4 commits behind head on main.

❗ Current head b9fc970 differs from pull request most recent head de8e1d6. Consider uploading reports for the commit de8e1d6 to get more accurate results

Files Patch % Lines
server/api/pipeline.go 0.00% 39 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3451      +/-   ##
==========================================
- Coverage   35.98%   35.92%   -0.07%     
==========================================
  Files         231      231              
  Lines       15553    15602      +49     
==========================================
+ Hits         5597     5605       +8     
- Misses       9539     9580      +41     
  Partials      417      417              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@qwerty287
Copy link
Contributor

@6543 can you take a look here? You requested changes so we cant merge without your approval

Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

yes thanks - my UX nit is addressed ❤️ !

@6543
Copy link
Member

6543 commented Apr 14, 2024

PS: sorry for the delay

@6543 6543 enabled auto-merge (squash) April 14, 2024 23:41
@6543 6543 merged commit b8763a8 into woodpecker-ci:main Apr 14, 2024
7 checks passed
@woodpecker-bot woodpecker-bot mentioned this pull request Apr 15, 2024
1 task
qwerty287 added a commit that referenced this pull request Apr 22, 2024
On top of #3451, addresses [PR
note](#3451 (comment))

related to #1100

Not tested.

---------

Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: qwerty287 <80460567+qwerty287@users.noreply.github.com>
Co-authored-by: qwerty287 <qwerty287@posteo.de>
@zc-devs zc-devs deleted the 3444-step-logs-remove branch June 24, 2024 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature add new functionality ui frontend related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to delete step's logs manually Expose log deletion for web UI too
6 participants