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

feat: add ability to delete specific tag #391

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
687 changes: 535 additions & 152 deletions package-lock.json

Large diffs are not rendered by default.

21 changes: 11 additions & 10 deletions package.json
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't add or change packages unless they are required for the implementation

Copy link
Author

Choose a reason for hiding this comment

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

just reverted it back, didn't realize i changed it sorry!!

Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,18 @@
"version": "0.1.0",
"private": true,
"dependencies": {
"@emotion/react": "^11.9.3",
"@emotion/styled": "^11.9.3",
"@mui/icons-material": "^5.2.5",
"@emotion/react": "^11.11.1",
"@emotion/styled": "^11.11.0",
"@mui/icons-material": "^5.14.13",
"@mui/lab": "^5.0.0-alpha.89",
"@mui/material": "^5.8.6",
"@mui/material": "^5.14.13",
"@mui/styles": "^5.8.6",
"@testing-library/jest-dom": "^5.16.1",
"@testing-library/react": "^12.1.2",
"@testing-library/user-event": "^13.5.0",
"axios": "^0.24.0",
"downshift": "^6.1.12",
"http-proxy-middleware": "^2.0.6",
"lodash": "^4.17.21",
"luxon": "^2.5.2",
"markdown-to-jsx": "^7.1.7",
Expand All @@ -24,14 +25,14 @@
"web-vitals": "^2.1.3"
},
"devDependencies": {
"@babel/plugin-proposal-private-property-in-object": "^7.16.7",
"@playwright/test": "^1.28.1",
"eslint": "^8.23.1",
"eslint-config-prettier": "^8.5.0",
"eslint-plugin-prettier": "^4.2.1",
"eslint": "^8.51.0",
"eslint-config-prettier": "^8.10.0",
"eslint-plugin-prettier": "^5.0.0",
"eslint-plugin-react": "^7.31.8",
"prettier": "^2.7.1",
"react-scripts": "^5.0.1",
"@babel/plugin-proposal-private-property-in-object": "^7.16.7"
"prettier": "^3.0.3",
"react-scripts": "^5.0.1"
},
"scripts": {
"start": "react-scripts start",
Expand Down
33 changes: 30 additions & 3 deletions src/components/Repo/RepoDetails.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// react global
import React, { useEffect, useMemo, useRef, useState } from 'react';

import { Delete as DeleteIcon } from '@mui/icons-material';
// external
import { DateTime } from 'luxon';
import { isEmpty, uniq } from 'lodash';
Expand Down Expand Up @@ -242,7 +242,29 @@
: `Timestamp N/A`;
return lastDate;
};

const handleDeleteRepo = () => {
const confirmed = window.confirm('Are you sure you want to delete this repo?');

Check warning on line 246 in src/components/Repo/RepoDetails.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Repo/RepoDetails.jsx#L246

Added line #L246 was not covered by tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

We implemend dialogs using MUI's dialog component, not the native window API, check FilterDialog used in the ExplorePage for example.

if (confirmed) {
const apiUrl = `http://localhost:3000/v2/${name}/manifests/`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

fetch(apiUrl, {

Check warning on line 249 in src/components/Repo/RepoDetails.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Repo/RepoDetails.jsx#L248-L249

Added lines #L248 - L249 were not covered by tests
method: 'DELETE'
})
.then((response) => {

Check warning on line 252 in src/components/Repo/RepoDetails.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Repo/RepoDetails.jsx#L252

Added line #L252 was not covered by tests
if (response.status === 202) {
// Repo deleted successfully
console.log('Repo deleted successfully');

Check warning on line 255 in src/components/Repo/RepoDetails.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Repo/RepoDetails.jsx#L255

Added line #L255 was not covered by tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't leave console logs inside code after finishing implementation

// You may want to navigate to another page or perform other actions as needed
} else {
console.log('Failed to delete the repo');

Check warning on line 258 in src/components/Repo/RepoDetails.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Repo/RepoDetails.jsx#L257-L258

Added lines #L257 - L258 were not covered by tests
// Handle the failure case here
}
})
.catch((error) => {
console.error('An error occurred:', error);

Check warning on line 263 in src/components/Repo/RepoDetails.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Repo/RepoDetails.jsx#L262-L263

Added lines #L262 - L263 were not covered by tests
// Handle any network or request error
});
}
};
return (
<>
{isLoading ? (
Expand Down Expand Up @@ -285,6 +307,11 @@
)}
</IconButton>
)}
{isAuthenticated() && (
<IconButton component="span" onClick={handleDeleteRepo} data-testid="delete-repo-button">
<DeleteIcon data-testid="delete-icon" />
</IconButton>
)}
</Stack>
<Typography gutterBottom className={classes.repoTitle}>
{repoDetailData?.title || 'Title not available'}
Expand Down Expand Up @@ -317,7 +344,7 @@
<Grid item xs={12} md={8} className={classes.tags}>
<Card className={classes.cardRoot}>
<CardContent className={classes.tagsContent}>
<Tags tags={tags} />
<Tags tags={tags} repoName={repoDetailData?.name} />
</CardContent>
</Card>
</Grid>
Expand Down
3 changes: 2 additions & 1 deletion src/components/Repo/Tabs/Tags.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const useStyles = makeStyles(() => ({

export default function Tags(props) {
const classes = useStyles();
const { tags } = props;
const { tags, repoName } = props;
const [tagsFilter, setTagsFilter] = useState('');
const [sortFilter, setSortFilter] = useState(tagsSortByCriteria.updateTimeDesc.value);

Expand All @@ -59,6 +59,7 @@ export default function Tags(props) {
return (
<TagCard
key={tag.tag}
repoName={repoName} //adding this
tag={tag.tag}
lastUpdated={tag.lastUpdated}
vendor={tag.vendor}
Expand Down
53 changes: 46 additions & 7 deletions src/components/Shared/TagCard.jsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import React, { useState } from 'react';
import { makeStyles } from '@mui/styles';
import { useNavigate } from 'react-router-dom';
import { Box, Card, CardContent, Collapse, Grid, Stack, Tooltip, Typography, Divider } from '@mui/material';
import { Box, Card, CardContent, Collapse, Grid, Stack, Tooltip, Typography, Divider, IconButton } from '@mui/material';
import { Markdown } from 'utilities/MarkdowntojsxWrapper';
import transform from 'utilities/transform';
import { DateTime } from 'luxon';
import { KeyboardArrowDown, KeyboardArrowRight } from '@mui/icons-material';
import { KeyboardArrowDown, KeyboardArrowRight, Delete as DeleteIcon } from '@mui/icons-material';

const useStyles = makeStyles((theme) => ({
card: {
Expand Down Expand Up @@ -79,7 +79,7 @@

export default function TagCard(props) {
const { repoName, tag, lastUpdated, vendor, manifests } = props;

console.log(props);
const [open, setOpen] = useState(false);
const classes = useStyles();

Expand All @@ -99,14 +99,53 @@
return (
<Card className={classes.card} raised>
<CardContent className={classes.content}>
<Typography variant="body1" align="left" className={classes.tagHeading}>
Tag
</Typography>
<div style={{ display: 'flex', alignItems: 'center' }}>
<Typography variant="body1" align="left" className={classes.tagHeading}>
Tag
</Typography>
<IconButton
color="primary"
style={{ marginLeft: 'auto' }}
onClick={() => {
Copy link
Collaborator

@raulkele raulkele Nov 15, 2023

Choose a reason for hiding this comment

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

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

const confirmed = window.confirm('Are you sure you want to perform this action?');

Check warning on line 110 in src/components/Shared/TagCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Shared/TagCard.jsx#L109-L110

Added lines #L109 - L110 were not covered by tests
if (confirmed) {
console.log('Button clicked and confirmed!');
console.log(props);
console.log('REPONAME: ', repoName);
const apiUrl = `http://localhost:3000/v2/${repoName}/manifests/${tag}`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

api.js for backend requests

fetch(apiUrl, {

Check warning on line 116 in src/components/Shared/TagCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Shared/TagCard.jsx#L112-L116

Added lines #L112 - L116 were not covered by tests
method: 'DELETE'
})
.then((response) => {

Check warning on line 119 in src/components/Shared/TagCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Shared/TagCard.jsx#L119

Added line #L119 was not covered by tests
if (response.status === 202) {
// Tag deleted successfully
console.log('Tag deleted successfully');

Check warning on line 122 in src/components/Shared/TagCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Shared/TagCard.jsx#L122

Added line #L122 was not covered by tests
// You may want to refresh the UI or perform other actions as needed
} else {
console.log('Failed to delete the tag');

Check warning on line 125 in src/components/Shared/TagCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Shared/TagCard.jsx#L124-L125

Added lines #L124 - L125 were not covered by tests
// Handle the failure case here
}
})
.catch((error) => {
console.error('An error occurred:', error);

Check warning on line 130 in src/components/Shared/TagCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Shared/TagCard.jsx#L129-L130

Added lines #L129 - L130 were not covered by tests
// Handle any network or request error
});
window.location.reload();
} else {

Check warning on line 134 in src/components/Shared/TagCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Shared/TagCard.jsx#L133-L134

Added lines #L133 - L134 were not covered by tests
// User canceled the action
console.log('Button click canceled.');

Check warning on line 136 in src/components/Shared/TagCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Shared/TagCard.jsx#L136

Added line #L136 was not covered by tests
}
console.log('Button clicked!');

Check warning on line 138 in src/components/Shared/TagCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Shared/TagCard.jsx#L138

Added line #L138 was not covered by tests
//should hold repo name and tag [line 119 and 120]
}}
>
<DeleteIcon />
</IconButton>
</div>
<Typography variant="body1" align="left" className={classes.tagName} onClick={() => goToTags()}>
{repoName && `${repoName}:`}
{tag}
</Typography>

<Stack sx={{ display: 'inline' }} direction="row" spacing={0.5}>
<Typography variant="caption" sx={{ fontWeight: '400', fontSize: '0.8125rem' }}>
Created
Expand Down
1 change: 0 additions & 1 deletion src/components/Tag/TagDetails.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ function TagDetails() {

// get url param from <Route here (i.e. image name)
const { reponame, tag } = useParams();

const classes = useStyles();

useEffect(() => {
Expand Down
4 changes: 2 additions & 2 deletions src/host.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const hostConfig = {
auto: true,
default: 'http://localhost:5000'
auto: false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't push this, keep only on local

default: 'http://localhost:8080'
};

const host = (manualHost = null) => {
Expand Down
11 changes: 11 additions & 0 deletions src/setupProxy.js
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what this is but it definitely shouldn't be here

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
const { createProxyMiddleware } = require('http-proxy-middleware');

Check warning on line 1 in src/setupProxy.js

View check run for this annotation

Codecov / codecov/patch

src/setupProxy.js#L1

Added line #L1 was not covered by tests

module.exports = function(app) {
  app.use(

Check warning on line 4 in src/setupProxy.js

View check run for this annotation

Codecov / codecov/patch

src/setupProxy.js#L3-L4

Added lines #L3 - L4 were not covered by tests
    '/v2',
    createProxyMiddleware({
      target: 'http://localhost:8080',
      changeOrigin: true,
    })
  );
};
Loading