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

add API endpoint to remove an app's image #568

Merged
merged 2 commits into from May 28, 2023

Conversation

tessus
Copy link
Contributor

@tessus tessus commented May 24, 2023

I have a few comments:

  1. I don't know if the swagger comments are generated or if I have to manually add them before the function.
  2. I am also not sure whether an error message makes sense, in case there is no image. IMO it should be a warning or info message, but not sure how to generate such a type.
  3. I am not a UI person. I have ideas, but how to realize them with HTML/CSS is another story. It would take me a few days to figure these things out. But I suggest that there is an overlay image (an x) in the bottom right corner of the icon when hovered over the icon in the frontend. Upon click, the API is called. (The onclick js I can most likely do, but no idea how to do the overlay image.)

Update: I will also write the test cases as soon as the specs are finished for the API call. Tests added.

@tessus tessus requested a review from a team as a code owner May 24, 2023 22:18
@codecov
Copy link

codecov bot commented May 24, 2023

Codecov Report

Patch coverage: 76.47% and project coverage change: -0.11 ⚠️

Comparison is base (a37afce) 86.09% compared to head (690464c) 85.99%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #568      +/-   ##
==========================================
- Coverage   86.09%   85.99%   -0.11%     
==========================================
  Files          45       45              
  Lines        1568     1585      +17     
==========================================
+ Hits         1350     1363      +13     
- Misses        137      139       +2     
- Partials       81       83       +2     
Impacted Files Coverage Δ
api/application.go 82.60% <75.00%> (-1.23%) ⬇️
router/router.go 88.11% <100.00%> (+0.11%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tessus tessus changed the title add API endpoint to remove a app's image add API endpoint to remove an app's image May 25, 2023
api/application.go Show resolved Hide resolved
api/application.go Show resolved Hide resolved
@tessus
Copy link
Contributor Author

tessus commented May 26, 2023

Please let me know, if anything is still missing.

api/application.go Outdated Show resolved Hide resolved
@jmattheis jmattheis merged commit 5cd2d54 into gotify:master May 28, 2023
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants