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: enable archive comment + wrap up #1275

Merged
merged 11 commits into from
Jun 23, 2023
Merged

feat: enable archive comment + wrap up #1275

merged 11 commits into from
Jun 23, 2023

Conversation

meowcodes
Copy link
Contributor

@meowcodes meowcodes commented Jun 21, 2023

enable archive comment + add archived comment ui
add clearing to draftjs editor (known issue by draftjs)
tried to make reactions unique in db but db will say some emojis are the same
using formik bc react doesnt like state change along with forwardref?
add commenting for tables

Screen.Recording.2023-06-21.at.4.58.27.PM.mov
Screenshot 2023-06-21 at 5 17 34 PM Screenshot 2023-06-21 at 4 59 28 PM

@meowcodes meowcodes requested review from czgu and jczhong84 June 21, 2023 20:47
querybook/server/logic/comment.py Outdated Show resolved Hide resolved
querybook/webapp/resource/comment.ts Outdated Show resolved Hide resolved
querybook/webapp/redux/comment/action.ts Outdated Show resolved Hide resolved
querybook/webapp/ui/Comment/Comment.tsx Outdated Show resolved Hide resolved
querybook/webapp/ui/Comment/Comments.tsx Outdated Show resolved Hide resolved
querybook/webapp/ui/Comment/Comments.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@czgu czgu left a comment

Choose a reason for hiding this comment

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

can you update the APIs so that archived comments aren't passed back to the user?

querybook/webapp/redux/comment/action.ts Outdated Show resolved Hide resolved
querybook/webapp/resource/comment.ts Outdated Show resolved Hide resolved
querybook/webapp/ui/Comment/Comment.tsx Outdated Show resolved Hide resolved
querybook/webapp/ui/Comment/Comments.tsx Outdated Show resolved Hide resolved
querybook/webapp/ui/Comment/Comments.tsx Outdated Show resolved Hide resolved
querybook/webapp/ui/Comment/Comments.tsx Outdated Show resolved Hide resolved
querybook/webapp/ui/RichTextEditor/RichTextEditor.tsx Outdated Show resolved Hide resolved
Comment on lines 70 to 85
"/comment/<int:comment_id>/text/",
methods=["PUT"],
)
def edit_comment(comment_id: int, **fields):
def edit_comment_text(comment_id: int, **fields):
assert_can_edit_and_delete(comment_id=comment_id)
return logic.edit_comment(comment_id=comment_id, **fields)


@register(
"/comment/<int:comment_id>/",
methods=["DELETE"],
"/comment/<int:comment_id>/archive/",
methods=["PUT"],
)
def remove_comment(comment_id: int):
def archive_comment(comment_id: int, **fields):
assert_can_edit_and_delete(comment_id=comment_id)
return logic.remove_comment(comment_id=comment_id)
logic.edit_comment(comment_id=comment_id, **fields)
return
Copy link
Collaborator

Choose a reason for hiding this comment

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

the comment update & soft delete in CommentResource are using the previous edit_comment endpoint, these two are not used.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am a little bit confused, why can't we use the same methods before, but change remove comment to soft delete?

Comment on lines 70 to 85
"/comment/<int:comment_id>/text/",
methods=["PUT"],
)
def edit_comment(comment_id: int, **fields):
def edit_comment_text(comment_id: int, **fields):
assert_can_edit_and_delete(comment_id=comment_id)
return logic.edit_comment(comment_id=comment_id, **fields)


@register(
"/comment/<int:comment_id>/",
methods=["DELETE"],
"/comment/<int:comment_id>/archive/",
methods=["PUT"],
)
def remove_comment(comment_id: int):
def archive_comment(comment_id: int, **fields):
assert_can_edit_and_delete(comment_id=comment_id)
return logic.remove_comment(comment_id=comment_id)
logic.edit_comment(comment_id=comment_id, **fields)
return
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am a little bit confused, why can't we use the same methods before, but change remove comment to soft delete?

querybook/server/datasources/comment.py Outdated Show resolved Hide resolved
querybook/webapp/redux/comment/action.ts Outdated Show resolved Hide resolved
querybook/webapp/redux/comment/reducer.ts Outdated Show resolved Hide resolved
querybook/webapp/ui/RichTextEditor/RichTextEditor.tsx Outdated Show resolved Hide resolved
querybook/webapp/ui/Comment/Comment.tsx Outdated Show resolved Hide resolved
archived: true,
};
draft.commentsById[commentId].archived = true;
draft.commentsById[commentId].text =
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: this line doesn't matter, I dont think we need to remove the text in user's redux yet

@czgu czgu merged commit 3b6a278 into pinterest:comment Jun 23, 2023
@meowcodes meowcodes deleted the end branch June 23, 2023 17:35
meowcodes added a commit to meowcodes/querybook that referenced this pull request Jun 23, 2023
* ui: fix richtext not clearing

* fix popover position issue

* archive

* clean

* clean

* add table

* update

* update

* update

* update

* update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants