Feature request/delete how tos and research articles

[KungRaseri]

PR Checklist

• [x] - Commit messages are descriptive, it will be used in our Release Notes

PR Type

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] Developer experience (improves developer workflows for contributing to the project)

Description

• [x] Add a Delete button to the How-To description section, next to the Edit button.
  • [x] Soft-delete with _deleted property set to true
  • [x] Filter deleted items out of how-tos list
  • [x] When viewing soft-deleted how-to, the delete button is disabled
  • [x] When viewing soft-deleted how-to, "* Marked for deletion" is clearly visible at the top of the section
• [x] Add a Delete button to the Research description section, next to the Edit button.
  • [x] Soft-delete with _deleted property set to true
  • [x] Filter deleted items out of research list
  • [x] When viewing soft-deleted research, the delete button is disabled
  • [x] When viewing soft-deleted research, "* Marked for deletion" is clearly visible at the top of the section
• [x] Add test coverage
  • [x] Unit testing (partial)
  • [x] E2E testing (partial)

Git Issues

Closes #2391

Screenshots

How-Tos:

The Button: The Modal: The Aftermath!

Research

The Button! The Modal! The Aftermath!

[codecov[bot]]

Codecov Report

Patch coverage: 54.21% and project coverage change: -4.82 :warning:

Comparison is base (a2031da) 48.39% compared to head (224c0e2) 43.57%.

:exclamation: Current head 224c0e2 differs from pull request most recent head 57f87e5. Consider uploading reports for the commit 57f87e5 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2438      +/-   ##
==========================================
- Coverage   48.39%   43.57%   -4.82%     
==========================================
  Files         254      259       +5     
  Lines        7082     7247     +165     
  Branches     1571     1591      +20     
==========================================
- Hits         3427     3158     -269     
- Misses       3624     4060     +436     
+ Partials       31       29       -2     

Impacted Files	Coverage Δ
...ontent/Howto/HowtoDescription/HowtoDescription.tsx	47.61% <37.50%> (-3.09%)	:arrow_down:
src/stores/Howto/howto.store.tsx	64.96% <47.05%> (-4.48%)	:arrow_down:
src/pages/Research/Content/ResearchDescription.tsx	68.96% <50.00%> (-10.58%)	:arrow_down:
src/stores/Research/research.store.tsx	73.38% <52.94%> (+0.10%)	:arrow_up:
src/utils/helpers.ts	63.04% <84.61%> (-31.16%)	:arrow_down:
src/pages/Research/Content/ResearchArticle.tsx	51.64% <100.00%> (-0.04%)	:arrow_down:

... and 87 files with indirect coverage changes

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[KungRaseri]

Started a discord discussion thread here with a few questions.

[KungRaseri]

Questions:

1. Should we redirect the user to /how-to and /research respectively after the delete action is completed?
2. Should we "cascade delete" all comments associated with a how-to/research article when it is deleted?
3. Should we limit the ability to delete to author and super-admin, or also include admin?

[thisislawatts]

@KungRaseri thanks for raising those questions 💜

1. Should we redirect the user to /how-to and /research respectively after the delete action is completed? That seems like a reasonable approach to me :)

2. Should we "cascade delete" all comments associated with a how-to/research article when it is deleted? I think we should be soft deleting how-to/research articles by adding a _deleted:true property to items which a User has opted to remove this will mean that:

• A platform admin or moderater can undelete in case of user error.
• Hard deleting of articles,how-tos and comments can be managed as an occasional background task. For example after an item has been marked as deleted for more than 30 days.

3. Should we limit the ability to delete to author and super-admin, or also include admin? The delete capability should be available for the original author, then admin and super-admin roles.

[KungRaseri]

@KungRaseri thanks for raising those questions 💜

1. Should we redirect the user to /how-to and /research respectively after the delete action is completed?
   That seems like a reasonable approach to me :)

2. Should we "cascade delete" all comments associated with a how-to/research article when it is deleted?
   I think we should be soft deleting how-to/research articles by adding a `_deleted:true` property to items which a User has opted to remove this will mean that:


* A platform admin or moderater can undelete in case of user error.

* Hard deleting of articles,how-tos and comments can be managed as an occasional background task. For example after an item has been marked as deleted for more than 30 days.


3. Should we limit the ability to delete to author and super-admin, or also include admin?
   The delete capability should be available for the original author, then admin and super-admin roles.

@thisislawatts Great! Thanks for the reply! Just a few follow ups for now:

1. With the _deleted property, would it be beneficial or necessary to add a _deletedBy property as well?
2. Should we setup another issue for the background task? or should we try to get that setup with this feature request?

[thisislawatts]

1. With the _deleted property, would it be beneficial or necessary to add a _deletedBy property as well?

I think that we should emit this data on the tracking event and log it, persisting it to the document seems unnecessary. Is there a specific scenario you had in mind that wouldnt be solved by event tracking/logs?

2. Should we setup another issue for the background task? or should we try to get that setup with this feature request?

No need for this at the moment, this seems like a future work item but I can't see when it would be done. So I think we trust that when/if it becomes important again someone will flag it.

[KungRaseri]

1. With the _deleted property, would it be beneficial or necessary to add a _deletedBy property as well?

I think that we should emit this data on the tracking event and log it, persisting it to the document seems unnecessary. Is there a specific scenario you had in mind that wouldnt be solved by event tracking/logs?

2. Should we setup another issue for the background task? or should we try to get that setup with this feature request?

No need for this at the moment, this seems like a future work item but I can't see when it would be done. So I think we trust that when/if it becomes important again someone will flag it.

1. Nope! Was a thought that came to mind and just wanted to bring it up :) I'll continue with logging/tracking and marking the how-to/research _deleted property for now.
2. Sounds good to me! Definitely going to want to discuss the process and everything before tackling it so wanted to make sure.

Another thought came to mind in regards to the deleted how-to/research articles and where they can be viewed:

1. Do we want the how-to/research articles that were marked for deletion to be viewable within the admin_v2/how-tos and admin_v2/research pages respectively? (at least until we get v1/v2 merged) Or do we want to hold off on that bit for now?

[AlfonsoGhislieri]

Just jumping in here @KungRaseri to say make sure you are using prettier as this is required to pass linting tests!

https://prettier.io/

[KungRaseri]

Just jumping in here @KungRaseri to say make sure you are using prettier as this is required to pass linting tests!

https://prettier.io/

@AlfonsoGhislieri Thanks for the reminder! I had it all setup, but didn't get to actually running it :)

I did have quite a few updates from it for files I haven't touched, which I'm sure is fine. Just wanted to give a heads up.

There also seemed to be an issue in the pk-template.handlebars file that I resolved.

I'll be circling back around and getting the unit tests and e2e test written now that prettier is working for me.

[KungRaseri]

This is ready for final review 👍 Thanks for all the feedback!

[AlfonsoGhislieri]

Also @KungRaseri it would be preferable if instead of merging you could use rebase instead. Keeps the commit history a lot cleaner. https://www.atlassian.com/git/tutorials/rewriting-history/git-rebase

Also could squash to group up commits and avoid a large number of commits. https://www.freecodecamp.org/news/git-squash-commits/

[KungRaseri]

Also @KungRaseri it would be preferable if instead of merging you could use rebase instead. Keeps the commit history a lot cleaner. https://www.atlassian.com/git/tutorials/rewriting-history/git-rebase

Also could squash to group up commits and avoid a large number of commits. https://www.freecodecamp.org/news/git-squash-commits/

Very new to this part of the process, but I rebased interactively to drop those mentioned commits. I'll look into it more as we go!

Thank you for the resources!

[cypress[bot]]

2 flaky tests on run #3748 ↗︎

0

102

0

0

2

Details:

Merge branch 'feature-request/delete-how-tos-and-research-articles' of https://g...
Project: onearmy-community-platform	Commit: 224c0e235c
Status: Passed	Duration: 04:06 💡
Started: Jun 30, 2023 12:17 AM	Ended: Jun 30, 2023 12:22 AM

  research/write.spec.ts • 1 flaky test • ci-chrome

View Output Video

Test		Artifacts
[Research] > [Edit a research article] > [By Owner]		Output Screenshots

  common.spec.ts • 1 flaky test • ci-chrome

View Output Video

Test		Artifacts
[Common] > [User Menu] > [By Authenticated]		Output Screenshots

_{This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.}

[thisislawatts]

@KungRaseri this is looking great, do you have some time to address the merge conflicts? I can take a look at this next week.

[KungRaseri]

@KungRaseri this is looking great, do you have some time to address the merge conflicts? I can take a look at this next week.

@thisislawatts Yeah, let's sync up after the 4th! Gonna be a busy weekend and beginning of the week :D

[thisislawatts]

Thanks for all the work on this @KungRaseri, I am going to adopt the changes here, resolve the merge conflicts and push them through to production. I will be sure to leave you as a co-author in the commit history. Thanks again ✨

[sigolenej]

Hey @thisislawatts I see this has been closed but has the issue been solved? Are humans now able to delete their HTs? :)