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

Notes 100: Comment Delete #1191

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
27 changes: 27 additions & 0 deletions cypress/e2e/notes-100/comment-delete.cy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import '@percy/cypress'
import {
homepageSetup,
returningUserVisitsHomepageWaitForModel,
auth0Login,
} from '../../support/utils'

/** {@link https://github.com/bldrs-ai/Share/issues/1187} */
describe('Notes 100: Comment delete', () => {
beforeEach(homepageSetup)
context('Returning user visits homepage', () => {
beforeEach(returningUserVisitsHomepageWaitForModel)
context('Open Notes > first note', () => {
beforeEach(() => {
cy.get('[data-testid="control-button-notes"]').click()
cy.get('[data-testid="list-notes"] :nth-child(1) > [data-testid="note-body"]').first().click()
})
it('Deleted comment disappears from the list', () => {
auth0Login()
cy.get(`[data-testid="list-notes"] > :nth-child(3)
> [data-testid="note-card"] > .MuiCardActions-root > .MuiBox-root > [data-testid="deleteComment"]`).click()
cy.get('[data-testid="list-notes"] > :nth-child(3) > [data-testid="note-card"]').should('not.contain', 'testComment_1')
cy.percySnapshot()
})
})
})
})
23 changes: 10 additions & 13 deletions src/Components/Notes/NoteCard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import {useAuth0} from '../../Auth0/Auth0Proxy'
import {
closeIssue,
updateIssue,
// TODO(pablo): deleteComment as deleteCommentGitHub,
} from '../../net/github/Issues'
import {
deleteComment,
} from '../../net/github/Comments'
import useStore from '../../store/useStore'
import {assertDefined} from '../../utils/assert'
import {getHashParamsFromHashStr, setHashParams} from '../../utils/location'
Expand Down Expand Up @@ -61,9 +63,8 @@ export default function NoteCard({
const notes = useStore((state) => state.notes)
const repository = useStore((state) => state.repository)
const selectedNoteId = useStore((state) => state.selectedNoteId)
// TODO(pablo)
// const comments = useStore((state) => state.comments)
// const setComments = useStore((state) => state.setComments)
const comments = useStore((state) => state.comments)
const setComments = useStore((state) => state.setComments)
const setNotes = useStore((state) => state.setNotes)
const setSelectedNoteId = useStore((state) => state.setSelectedNoteId)
const setSelectedNoteIndex = useStore((state) => state.setSelectedNoteIndex)
Expand Down Expand Up @@ -158,16 +159,11 @@ export default function NoteCard({
* @param {string} accessToken
* @param {number} commentId
*/
// TODO(pablo)
/* async function deleteComment(commentId) {
// TODO(pablo): handle response
await deleteCommentGitHub(repository, commentId, accessToken)
const newComments = comments.map((comment) => ({
...comment,
synched: (comment.id !== commentId) && comment.synched,
}))
async function deleteCommentGithub(commentId) {
await deleteComment(repository, commentId, accessToken)
Copy link
Member

Choose a reason for hiding this comment

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

I think we planned to use return statuses to set comments instead of filtering, like in #1200

Copy link
Member Author

Choose a reason for hiding this comment

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

@pablo-mayrgundter getting 204 with the data of the response object undefined.

ChatGPT:
A 204 No Content status code indicates that the server successfully processed the request, but is not returning any content. This is typical for a POST request where the server does not return a response body.

const newComments = comments.filter((comment) => comment.id !== commentId)
setComments(newComments)
} */
}


/** Update issue on GH, set read-only */
Expand Down Expand Up @@ -227,6 +223,7 @@ export default function NoteCard({
selectCard={selectCard}
selected={selected}
submitUpdate={submitUpdate}
deleteComment={deleteCommentGithub}
synched={synched}
username={username}
/>
Expand Down
49 changes: 32 additions & 17 deletions src/Components/Notes/NoteFooter.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import AddCommentOutlinedIcon from '@mui/icons-material/AddCommentOutlined'
import CheckIcon from '@mui/icons-material/Check'
import ForumOutlinedIcon from '@mui/icons-material/ForumOutlined'
import GitHubIcon from '@mui/icons-material/GitHub'
import DeleteOutlineIcon from '@mui/icons-material/DeleteOutline'
import PhotoCameraIcon from '@mui/icons-material/PhotoCamera'
import CameraIcon from '../../assets/icons/Camera.svg'
import PlaceMarkIcon from '../../assets/icons/PlaceMark.svg'
Expand All @@ -23,6 +24,7 @@ import ShareIcon from '../../assets/icons/Share.svg'
*/
export default function NoteFooter({
accessToken,
deleteComment,
editMode,
embeddedCameras,
id,
Expand Down Expand Up @@ -63,13 +65,14 @@ export default function NoteFooter({
'_blank')
}


return (
<CardActions>
{isNote &&
<TooltipIconButton
title='Open in Github'
size='small'
placement='bottom'
placement='top'
onClick={openGithubIssue}
icon={<GitHubIcon className='icon-share'/>}
aboutInfo={false}
Expand All @@ -80,7 +83,7 @@ export default function NoteFooter({
<TooltipIconButton
title='Show the camera view'
size='small'
placement='bottom'
placement='top'
onClick={onClickCamera}
icon={<CameraIcon className='icon-share'/>}
aboutInfo={false}
Expand All @@ -90,7 +93,7 @@ export default function NoteFooter({
<TooltipIconButton
title='Share'
size='small'
placement='bottom'
placement='top'
onClick={() => {
onClickShare()
setShareIssue(!shareIssue)
Expand All @@ -113,7 +116,7 @@ export default function NoteFooter({
<TooltipIconButton
title='Place Mark'
size='small'
placement='bottom'
placement='top'
onClick={() => {
togglePlaceMarkActive(id)
}}
Expand All @@ -130,48 +133,60 @@ export default function NoteFooter({
<TooltipIconButton
title='Take Screenshot'
size='small'
placement='bottom'
placement='top'
onClick={() => {
setScreenshotUri(viewer.takeScreenshot())
}}
icon={<PhotoCameraIcon className='icon-share'/>}
/>
}

{editMode &&
<TooltipIconButton
title='Save'
placement='left'
icon={<CheckIcon className='icon-share'/>}
onClick={() => submitUpdate(repository, accessToken, id)}
/>
}

{isNote && !selected &&
<TooltipIconButton
title='Add Comment'
size='small'
placement='bottom'
placement='top'
selected={showCreateComment}
onClick={selectCard}
icon={<AddCommentOutlinedIcon className='icon-share'/>}
/>
}
{editMode &&
<Box sx={{marginLeft: 'auto', padding: '0 0.5em'}}>
Copy link
Member

Choose a reason for hiding this comment

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

Could the left-padding be moved to the parent to remove it from have it repeated in these 3 components?

<TooltipIconButton
title='Save'
placement='top'
icon={<CheckIcon className='icon-share'/>}
onClick={() => submitUpdate(repository, accessToken, id)}
/>
</Box>
}

{numberOfComments > 0 && !editMode &&
<Box sx={{marginLeft: 'auto', padding: '0 0.5em'}}>
{!selected &&
<TooltipIconButton
title='Discussion'
size='small'
placement='bottom'
placement='top'
onClick={selectCard}
icon={<ForumOutlinedIcon className='icon-share'/>}
/>
}
{!selected && numberOfComments}
{!selected && numberOfComments}
</Box>
}
{!isNote && user && username === user.nickname &&
<Box sx={{marginLeft: 'auto', padding: '0 0 0 0.5em'}}>
<TooltipIconButton
title='Delete'
placement='top'
buttonTestId='deleteComment'
icon={<DeleteOutlineIcon className='icon-share'/>}
onClick={() => deleteComment(id)}
/>
</Box>
}
</CardActions>
)
}
1 change: 0 additions & 1 deletion src/Components/Notes/Notes.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export default function Notes() {
const repository = useStore((state) => state.repository)
const selectedNoteId = useStore((state) => state.selectedNoteId)
const setComments = useStore((state) => state.setComments)

const [hasError, setHasError] = useState(false)

const {user} = useAuth0()
Expand Down
2 changes: 1 addition & 1 deletion src/__mocks__/api-handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ function githubHandlers(githubStore) {
rest.delete(`${GH_BASE}/repos/:org/:repo/issues/comments/:commentId`, (req, res, ctx) => {
const {org, repo, commentId} = req.params

if (org !== 'bldrs-ai' || repo !== 'Share' || !commentId) {
if (org !== 'pablo-mayrgundter' || repo !== 'Share' || !commentId) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's move these to one of the mock test IDs to make it clear that it's not prod data, and also to free my ghost from the machine :)

return res(ctx.status(httpNotFound))
}

Expand Down
4 changes: 2 additions & 2 deletions src/net/github/Comments.fixture.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export const MOCK_COMMENTS = {
node_id: 'IC_kwDOFwgxOc5EPlQ3',
number: 1,
user: {
login: 'OlegMoshkovich',
login: 'cypresstester',
id: 3433607,
node_id: 'MDQ6VXNlcjM0MzM2MDY=',
avatar_url: 'https://avatars.githubusercontent.com/u/3433606?v=4',
Expand Down Expand Up @@ -56,7 +56,7 @@ export const MOCK_COMMENTS = {
number: 2,
node_id: 'IC_kwDOFwgxOc5EPlQ3',
user: {
login: 'OlegMoshkovich',
login: 'cypresstester',
id: 3433606,
node_id: 'MDQ6VXNlcjM0MzM2MDY=',
avatar_url: 'https://avatars.githubusercontent.com/u/3433606?v=4',
Expand Down
2 changes: 1 addition & 1 deletion src/net/github/Comments.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe('net/github/Comments', () => {
})

it('successfully delete comment', async () => {
const res = await deleteComment({orgName: 'bldrs-ai', name: 'Share'}, 1)
const res = await deleteComment({orgName: 'pablo-mayrgundter', name: 'Share'}, 1)
Copy link
Member

Choose a reason for hiding this comment

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

same

expect(res.status).toEqual(httpOK)
})
})
Loading