feat: add ability to delete specific tag #391
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #391 +/- ##
==========================================
- Coverage 86.73% 84.46% -2.27%
==========================================
Files 53 54 +1
Lines 1553 1590 +37
Branches 406 415 +9
==========================================
- Hits 1347 1343 -4
- Misses 193 235 +42
+ Partials 13 12 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Please don't add or change packages unless they are required for the implementation
There was a problem hiding this comment.
just reverted it back, didn't realize i changed it sorry!!
| @@ -242,7 +242,29 @@ function RepoDetails() { | |||
| : `Timestamp N/A`; | |||
| return lastDate; | |||
| }; | |||
|
|
|||
| const handleDeleteRepo = () => { | |||
| const confirmed = window.confirm('Are you sure you want to delete this repo?'); | |||
There was a problem hiding this comment.
We implemend dialogs using MUI's dialog component, not the native window API, check FilterDialog used in the ExplorePage for example.
| const handleDeleteRepo = () => { | ||
| const confirmed = window.confirm('Are you sure you want to delete this repo?'); | ||
| if (confirmed) { | ||
| const apiUrl = `http://localhost:3000/v2/${name}/manifests/`; |
There was a problem hiding this comment.
We declare our endpoints inside the api.js file. and send requests using axios not fetch. Check the api.js file and usage of api thought the project for examples.
Also, the backend host is declared in the hosts.js file, don't hardcode it inside code
| .then((response) => { | ||
| if (response.status === 202) { | ||
| // Repo deleted successfully | ||
| console.log('Repo deleted successfully'); |
There was a problem hiding this comment.
don't leave console logs inside code after finishing implementation
| <IconButton | ||
| color="primary" | ||
| style={{ marginLeft: 'auto' }} | ||
| onClick={() => { |
There was a problem hiding this comment.
In general, if your function is longer than 1 line, you want to declare it outside the JSX. Also, same thing about using the window.confirm api
| console.log('Button clicked and confirmed!'); | ||
| console.log(props); | ||
| console.log('REPONAME: ', repoName); | ||
| const apiUrl = `http://localhost:3000/v2/${repoName}/manifests/${tag}`; |
There was a problem hiding this comment.
api.js for backend requests
| @@ -1,6 +1,6 @@ | |||
| const hostConfig = { | |||
| auto: true, | |||
| default: 'http://localhost:5000' | |||
| auto: false, | |||
There was a problem hiding this comment.
don't push this, keep only on local
There was a problem hiding this comment.
I'm not sure what this is but it definitely shouldn't be here
|
Also, you need to squash your commits so you only have 1 |
|
Superseded by #401 |
|
@eviishondell Thanks for your PR. |
No problem! |
What type of PR is this?
Which issue does this PR fix:
What does this PR do / Why do we need it:
If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:
Testing done on this change:
Automation added to e2e:
Will this break upgrades or downgrades. Has updating a running cluster been tested?:
Does this change require updates to the CNI daemonset config files to work?:
Does this PR introduce any user-facing change?:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.