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.

Sign up for GitHub

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

Jump to bottom

Add "in reply to" references to comments #21237

Merged
merged 2 commits into from
Nov 7, 2024
Merged

Add "in reply to" references to comments #21237

merged 2 commits into from
Nov 7, 2024

Conversation

kevinansfield
Copy link
Member

@kevinansfield kevinansfield commented Oct 6, 2024 •
edited
Loading

ref https://linear.app/tryghost/issue/PLG-230

  • added comments.in_reply_to_id column to database
  • added in-reply-to support to comments API
    • adds in_reply_to_id to API output
    • adds in_reply_to_snippet to API output
      • dynamically generated from the HTML of the replied-to comment
      • excluded if the replied-to comment has been deleted or hidden
    • allows setting in_reply_to_id when creating comments
      • id must reference a reply with the same parent
      • id must reference a published comment
    • adds email notification for the original reply author when their comment is replied to

TODO

  • unit test in_reply_to_snippet part of comments data mapper

@github-actions github-actions bot added the migration [pull request] Includes migration for review label Oct 6, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 6, 2024 •
edited by ronaldlangeveld
Loading

It looks like this PR contains a migration 👀
Here's the checklist for reviewing migrations:

General requirements

  • Satisfies idempotency requirement (both up() and down())
  • Does not reference models
  • Filename is in the correct format (and correctly ordered)
  • Targets the next minor version
  • All code paths have appropriate log messages
  • Uses the correct utils
  • Contains a minimal changeset
  • Does not mix DDL/DML operations
  • Tested in MySQL and SQLite

Schema changes

  • Both schema change and related migration have been implemented
  • For index changes: has been performance tested for large tables
  • For new tables/columns: fields use the appropriate predefined field lengths
  • For new tables/columns: field names follow the appropriate conventions
  • Does not drop a non-alpha table outside of a major version

Data changes

  • Mass updates/inserts are batched appropriately
  • Does not loop over large tables/datasets
  • Defends against missing or invalid data
  • For settings updates: follows the appropriate guidelines

@kevinansfield kevinansfield added deploy-to-staging and removed migration [pull request] Includes migration for review labels Oct 6, 2024
@Ghost-Slimer
Copy link

Deployed to staging with ID: 3098

@Ghost-Slimer
Copy link

Deployed to staging with ID: 3098

@kevinansfield kevinansfield force-pushed the comments-replied-to-ref branch 2 times, most recently from b064d44 to 60ffd8e Compare October 13, 2024 14:33
@Ghost-Slimer
Copy link

Deployed to staging with ID: 3098

@Ghost-Slimer
Copy link

Deployed to staging with ID: 3098

@Ghost-Slimer
Copy link

Deployed to staging with ID: 3098

@kevinansfield
Added comments.in_reply_to_id column migration
8b5f278 
ref https://linear.app/tryghost/issue/PLG-230

- `comments.in_reply_to_id` will be used to keep a reference to the comment that the new comment was directed at
- used only for replies-to-replies, will be `null` for the top-level parent and `null` for any replies directly to that parent
- technically allows for infinite nesting within a parent comment thread but we won't be using that ability for now
- `comments.parent_id` will be kept as it provides a useful optimisation for loading the top-level comments list
- we're not using `comments.parent_id` for this to keep complexity down and avoid the need for recursive lookups
@kevinansfield
Added in-reply-to support to comments API
f66c244 
ref https://linear.app/tryghost/issue/PLG-230

- adds `in_reply_to_id` to API output
- adds `in_reply_to_snippet` to API output
  - dynamically generated from the HTML of the replied-to comment
  - excluded if the replied-to comment has been deleted or hidden
- adds `commentSnippet` to `@tryghost/html-to-plaintext`
  - skips anchor tag URLs as they won't be useful for snippet purposes
  - skips blockquotes so the snippet is more likely to contain the unique content of the replied-to comment when it's quoting a previous comment
  - returns a single line (no newline chars)
- allows setting `in_reply_to_id` when creating comments
  - id must reference a reply with the same parent
  - id must reference a published comment
- adds email notification for the original reply author when their comment is replied to
@Ghost-Slimer
Copy link

Deployed to staging with ID: 3098

@kevinansfield kevinansfield merged commit 79f41dc into main Nov 7, 2024
21 checks passed
@kevinansfield kevinansfield deleted the comments-replied-to-ref branch November 7, 2024 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
deploy-to-staging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kevinansfield @Ghost-Slimer