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

fix(r/demo/boards): correctly render reposts #1530

Merged
merged 3 commits into from
Feb 1, 2024

Conversation

jefft0
Copy link
Contributor

@jefft0 jefft0 commented Jan 12, 2024

r/demo/boards already implements the data structures for reposts and the API function CreateRepost. But the Render display doesn't handle reposts correctly. I did a repost on test3 with:

gnokey maketx call -pkgpath "gno.land/r/demo/boards" -func "CreateRepost" -gas-fee 1000000ugnot -gas-wanted 5000000 -send "" -broadcast -chainid "test3" -args "1" -args "2" -args "" -args "check out this NFT post" -args "3" -remote "test3.gno.land:36657" jefft0

This should display on board number 3 "boardtest". https://test3.gno.land/r/demo/boards:boardtest . Indeed, it shows

check out this NFT post - @jefft0, 2024-01-12 4:24pm UTC [x] (0 replies)

But when I click on the date to see the post, the page shows "reply does not exist with id: 2" because Render incorrectly treats a repost as a reply.

This pull request is one approach to displaying reposts. It has three commits. The first commit adds GetRepostFormURL (similar to GetReplyFormURL) and updates RenderPost with a helper for [repost] (similar to [reply]). It also updates RenderSummary to check if the post is a repost and displays the reposted post along with the comment for the repost, and an indicator like "(2 reposts)". With this change, it shows something like the following where "mytitle" is the title of the original post with its body.

Screenshot 2024-01-12 at 17 50 19

The second commit updates existing tests to have the correct expected output.

The third commit adds a tests for CreateRepost with an expected output and errors.

(Why did we bother with fixing reposts? Because we implemented reposts in GnoSocial which is a modification of r/demo/boards. Since we fixed this in GnoSocial it's easy to contribute the same fix here, and maybe get some good feedback on the approach.)

Contributors' checklist...
  • Added new tests, or not needed, or not feasible
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description

@jefft0 jefft0 requested a review from a team as a code owner January 12, 2024 16:41
@github-actions github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Jan 12, 2024
Copy link

codecov bot commented Jan 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f6698e6) 56.10% compared to head (5c82d64) 55.86%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1530      +/-   ##
==========================================
- Coverage   56.10%   55.86%   -0.25%     
==========================================
  Files         432      431       -1     
  Lines       66076    65834     -242     
==========================================
- Hits        37075    36775     -300     
- Misses      26104    26178      +74     
+ Partials     2897     2881      -16     
Flag Coverage Δ
go-1.21.x ∅ <ø> (∅)
misc ∅ <ø> (∅)
misc-_test.genstd ∅ <ø> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jefft0 jefft0 changed the title chore: boards show reposts chore: r/demo/boards: show reposts Jan 12, 2024
Signed-off-by: Jeff Thompson <jeff@thefirst.org>
…reposts

Signed-off-by: Jeff Thompson <jeff@thefirst.org>
Signed-off-by: Jeff Thompson <jeff@thefirst.org>
@zivkovicmilos zivkovicmilos requested a review from a team January 22, 2024 12:34
Copy link
Contributor

@leohhhn leohhhn left a comment

Choose a reason for hiding this comment

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

Looks good, I have nothing special to add as feedback :)

Copy link
Member

@thehowl thehowl left a comment

Choose a reason for hiding this comment

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

other than the nit, looks good

@thehowl thehowl changed the title chore: r/demo/boards: show reposts fix(r/demo/boards): correctly render reposts Jan 31, 2024
@thehowl thehowl merged commit bcb3f5d into gnolang:master Feb 1, 2024
187 of 188 checks passed
leohhhn pushed a commit to leohhhn/gno that referenced this pull request Feb 2, 2024
r/demo/boards already implements the data structures for reposts and the
API function
[CreateRepost](https://github.com/gnolang/gno/blob/a1c9a8c142038a9624a035262b899a24e16ff3cd/examples/gno.land/r/demo/boards/public.gno#L87).
But the Render display doesn't handle reposts correctly. I did a repost
on test3 with:
```
gnokey maketx call -pkgpath "gno.land/r/demo/boards" -func "CreateRepost" -gas-fee 1000000ugnot -gas-wanted 5000000 -send "" -broadcast -chainid "test3" -args "1" -args "2" -args "" -args "check out this NFT post" -args "3" -remote "test3.gno.land:36657" jefft0
```

This should display on board number 3 "boardtest".
https://test3.gno.land/r/demo/boards:boardtest . Indeed, it shows
```
check out this NFT post - @jefft0, 2024-01-12 4:24pm UTC [x] (0 replies)
```

But when I click on the date to see the post, [the
page](https://test3.gno.land/r/demo/boards:boardtest/2/2) shows "reply
does not exist with id: 2" because Render incorrectly treats a repost as
a reply.

This pull request is one approach to displaying reposts. It has three
commits. The first commit adds `GetRepostFormURL` (similar to
`GetReplyFormURL`) and updates `RenderPost` with a helper for [repost]
(similar to [reply]). It also updates `RenderSummary` to check if the
post is a repost and displays the reposted post along with the comment
for the repost, and an indicator like "(2 reposts)". With this change,
it shows something like the following where "mytitle" is the title of
the original post with its body.

![Screenshot 2024-01-12 at 17 50
19](https://github.com/gnolang/gno/assets/1999543/48c0ec39-3716-45e4-b20d-bc01b900ea8f)

The second commit updates existing tests to have the correct expected
output.

The third commit adds a tests for `CreateRepost` with an expected output
and errors.

(Why did we bother with fixing reposts? Because we implemented reposts
in GnoSocial which is a modification of r/demo/boards. Since we fixed
this in GnoSocial it's easy to contribute the same fix here, and maybe
get some good feedback on the approach.)

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description

---------

Signed-off-by: Jeff Thompson <jeff@thefirst.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧾 package/realm Tag used for new Realms or Packages.
Projects
Status: Done
Status: No status
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants