Feat: #482 when Reset topic clicked it should delete comments too #580
Conversation
… too Added a function which will delete comments when Reset topic button clicked
👷 Deploy request for velvety-vacherin-4193fb pending review.Visit the deploys page to approve it
|
✅ Deploy Preview for ameliorate-docs canceled.
|
There was a problem hiding this comment.
Found the issue, see the other PR comments.
Also: I'm assuming this PR is intended to help figure out the bug. FYI for cases like this, you can use a PR draft in order to convey that the PR isn't intended to be thoroughly reviewed for merge yet.
|
|
||
| useCommentStore.apiSyncer.pause(); |
There was a problem hiding this comment.
These are changes I made in a separate commit - it seems that when you rebased, you removed these changes. This might separately cause problems for you. The easiest way to fix this right now is probably to just manually add these lines back in, and create a fix commit with that.
What I suspect happened is that when you pulled and saw conflicts, you clicked "accept current changes", which kept your changes and removed the changes being pulled in. In this case, you'd have wanted to keep both your changes and the changes being pulled in (generally if you pull changes that someone else made, you probably want to keep them).
To avoid this in the future, it might help to read up on handling merge conflicts and why/how to rebase. Personally, I think Philomatics does a fantastic job of explaining these concepts:
- video on handling merge conflicts
- video on why/how to rebase & difference between rebasing vs merging (note: video says not to rebase if you've pushed your branch, but really you can safely do that as long as nobody else is making changes on your branch)
Additionally, I would fixup (i.e. squash without rewording the original commit's message) said fix commit into your original commit, so that your commit only has your changes, and the commit history is a little cleaner. But you don't have to fixup because I'll handle squashing all commits before merging anyway. If you'd like to look into this, here's another great Philomatics video explaining the interactive rebase, which makes it easy to modify your commit history.
There was a problem hiding this comment.
Hey @keyserj , really appreciate you sharing the Philomatics YouTube channel. It was super helpful!
In my last commit, I made a pull request, then committed my changes, and pushed them.
I think you’re right about your note. When I did the pull request, I hit "accept current changes" because I wanted to include my changes, thinking that the other changes would merge in too.
There was a problem hiding this comment.
The easiest way to fix this right now is probably to just manually add these lines back in, and create a fix commit with that.
Are you suggesting I should edit the git files in the browser or in VSCode and then push it again?
There was a problem hiding this comment.
When I did the pull request, I hit "accept current changes" because I wanted to include my changes, thinking that the other changes would merge in too.
One tip I have is to look at the diff between your branch and the upstream after you've resolved conflicts - the diff should only have your changes, not anyone else's (unless you're aware of their changes and intentionally want to change them). You can use a PR to see this diff or use vscode's gitlens extension to compare HEAD (your latest commit on your checked out branch) with upstream/main to see the same without a PR, like so:
Code_6OHRbZCwXk.mp4
Are you suggesting I should edit the git files in the browser or in VSCode and then push it again?
Generally I would make changes locally (via vscode) so that I can test them before pushing anyway (I'm also less familiar with the browser interface). But in either case you'll need to create a new commit for it (or modify the existing commit, but that's not necessary). Also note that if you use the browser, you'll need to remember to pull those changes locally if you do any further local development.
| @@ -160,6 +160,10 @@ export const deleteComment = (commentId: string) => { | |||
| ); | |||
| }; | |||
|
|
|||
| export const resetComment = () => { | |||
| useCommentStore.setState({ ...initialState, comments: [] }, false, "resetComment"); | |||
There was a problem hiding this comment.
Here is the main cause of the error you're seeing: we want to reset to the initial state except for the initial state's topic value, similar to how resetTopicData() is implemented. This is from an unfortunate bit of tech debt 😅 I'm hoping to remove it soon in #581. Essentially if you reset the Topic, we won't know which Topic to make an API request for, preventing the changes from being saved properly.
Side note: you also don't need to set comments: [] here - initialState has that in it already.
Closes #[482]
Description of changes
Additional context
Feat.-.482.mp4