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

Decode Content Clean-up #9251

Merged
merged 18 commits into from
Oct 9, 2024
Merged

Decode Content Clean-up #9251

merged 18 commits into from
Oct 9, 2024

Conversation

timolegros
Copy link
Collaborator

@timolegros timolegros commented Sep 17, 2024

Link to Issue

Closes: #9184

Description of Changes

  • Removed plaintext and body_backup columns

"How We Fixed It"

Test Plan

  1. Create threads with a variety of formats (using markdown) and ensure the thread previews appear normal
    Important: Ensure Thread preview on mobile appears broken #8611 is not reintroduced.
    Context:

      // Future Ref: this fixes https://github.com/hicommonwealth/commonwealth/issues/8611 for iOS mobile
      // where quill renders broken/cut-off/overlapping thread.plaintext in cases when there are multiple
      // <p/> tags in the quill delta for thread.plaintext or if thread.plaintext has \n characters which
      // iOS devices don't seem to render correctly.
    

    I replaced thread.plaintext.replaceAll(/\n/g, '').slice(0, 150) with thread.body which is raw markdown that may contain /n.

  2. Using the previously created thread, create a snapshot proposal. Images should be removed but all markdown should be properly rendered.

Deployment Plan

Other Considerations

@timolegros
Copy link
Collaborator Author

This PR is technically ready (minus any conflicts) but cannot be merged until the decoding scripts from #9183 are executed in production. Will leave this PR in draft until then.

# Conflicts:
#	libs/model/src/comment/UpdateComment.command.ts
#	libs/model/src/thread/UpdateThread.command.ts
#	packages/commonwealth/client/scripts/state/api/comments/deleteComment.ts
#	packages/commonwealth/scripts/decode-db-content.ts
#	packages/commonwealth/server/routes/feed.ts
@timolegros timolegros marked this pull request as ready for review October 2, 2024 06:20
@timolegros
Copy link
Collaborator Author

@mzparacha tagging you here for review to ensure we don't regress on #8611

@timolegros timolegros force-pushed the tim/decode-content-clean-up branch from 6214721 to 4425189 Compare October 2, 2024 08:13
# Conflicts:
#	packages/commonwealth/client/scripts/views/pages/discussions/ThreadCard/ThreadCard.tsx
@mzparacha
Copy link
Contributor

@mzparacha tagging you here for review to ensure we don't regress on #8611

sry I missed this, will review early tomorrow

@mzparacha
Copy link
Contributor

Tested #8611 and it works well for me on iOS safari. Would like @masvelio to take a look as well.

  1. Found an unformattable markdown error using markdown from here (probably an editor issue - not directly related to this PR cc: @burtonator)
image
  1. Getting some raw quill deltas rendered in home feed
image

cc: @timolegros

@mzparacha
Copy link
Contributor

Update: 2 from #9251 (comment) is resolved after running content decode scripts (TY @timolegros). I wasn't able to repro 1 from #9251 (comment) in the master branch + the markdown doesn't render there correctly cc: @burtonator

image

Copy link
Contributor

@mzparacha mzparacha left a comment

Choose a reason for hiding this comment

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

The markdown bug was with the new editor, everything works well. TY for all the changes

@timolegros timolegros merged commit 270c485 into master Oct 9, 2024
10 checks passed
@timolegros timolegros deleted the tim/decode-content-clean-up branch October 9, 2024 15:47
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.

Decoding Clean-up
4 participants