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 empty links causing confusing thread hydration behaviour #55

Merged
merged 4 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions app/services/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,10 @@ func (s *service) Archive(ctx context.Context, slug datagraph.ClusterSlug) (*dat

func (s *service) hydrateLink(ctx context.Context, partial Partial) (opts []cluster.Option) {
text, textOK := partial.Content.Get()
url, urlOK := partial.URL.Get()

if !textOK && !urlOK {
if !textOK && !partial.URL.Ok() {
return
}

return s.hydrator.HydrateCluster(ctx, text, url)
return s.hydrator.HydrateCluster(ctx, text, partial.URL)
}
11 changes: 5 additions & 6 deletions app/services/hydrator/extractor/destructure.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ func Destructure(markdown string) EnrichedProperties {
var walk func(n ast.Node)
walk = func(n ast.Node) {
switch node := n.(type) {
case *ast.Link:
if parsed, err := url.Parse(string(node.Destination)); err == nil {
links = append(links, parsed.String())
}

case *ast.Text:
if len(node.Literal) == 0 {
return
Expand All @@ -42,12 +47,6 @@ func Destructure(markdown string) EnrichedProperties {
textonly.Write(oneline)
textonly.WriteByte(' ')

if strings.HasPrefix(string(oneline), "http") {
if parsed, err := url.Parse(string(oneline)); err == nil {
links = append(links, parsed.String())
}
}

default:
container := n.AsContainer()
if container == nil {
Expand Down
63 changes: 63 additions & 0 deletions app/services/hydrator/extractor/destructure_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package extractor

import (
"testing"

"github.com/stretchr/testify/assert"
)

const md1 = `Check out my new oven!

![http://localhost:3000/api/v1/assets/5902d45bf0cd23d88c70b5e38652c44e2d815b08](http://localhost:3000/api/v1/assets/5902d45bf0cd23d88c70b5e38652c44e2d815b08)

Isn't it cool?

Here's a link: https://ao.com/cooking/ovens`

const md2 = `Embeds, separate line:

https://ao.com/cooking/ovens

Same line: https://x.com/southclaws bare link.

Same line, [link text](https://cla.ws) inline.
`

func TestDestructure(t *testing.T) {
tests := []struct {
name string
text string
want EnrichedProperties
}{
{
name: "ovens_lol",
text: md1,
want: EnrichedProperties{
Short: "Check out my new oven! http://localhost:3000/api/v1/assets/5902d45bf0cd23d88c70b5e38652c44e2d815b08 Isn't it cool? Here's a link...",
Links: []string{
"https://ao.com/cooking/ovens",
},
},
},
{
name: "embedsssss",
text: md2,
want: EnrichedProperties{
Short: "Embeds, separate line: Same line: bare link. Same line, inline.",
Links: []string{
"https://ao.com/cooking/ovens",
"https://x.com/southclaws",
"https://cla.ws",
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ep := Destructure(tt.text)

assert.Equal(t, tt.want.Short, ep.Short)
assert.Equal(t, tt.want.Links, ep.Links)
})
}
}
7 changes: 7 additions & 0 deletions app/services/hydrator/fetcher/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/Southclaws/fault"
"github.com/Southclaws/fault/fctx"
"github.com/Southclaws/fault/ftag"
"go.uber.org/fx"
"go.uber.org/zap"

Expand All @@ -18,6 +19,8 @@ import (
"github.com/Southclaws/storyden/app/services/url"
)

var errEmptyLink = fault.New("empty link")

type Service interface {
Fetch(ctx context.Context, url string) (*link.Link, error)
}
Expand Down Expand Up @@ -50,6 +53,10 @@ func New(
}

func (s *service) Fetch(ctx context.Context, url string) (*link.Link, error) {
if url == "" {
return nil, fault.Wrap(errEmptyLink, fctx.With(ctx), ftag.With(ftag.InvalidArgument))
}

r, err := s.lr.Search(ctx, link.WithURL(url))
if err != nil {
return nil, fault.Wrap(err, fctx.With(ctx))
Expand Down
21 changes: 11 additions & 10 deletions app/services/hydrator/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"go.uber.org/fx"
"go.uber.org/zap"

"github.com/Southclaws/opt"
"github.com/Southclaws/storyden/app/resources/asset"
"github.com/Southclaws/storyden/app/resources/cluster"
"github.com/Southclaws/storyden/app/resources/item"
Expand All @@ -17,10 +18,10 @@ import (
)

type Service interface {
HydrateThread(ctx context.Context, body, url string) []thread.Option
HydrateReply(ctx context.Context, body, url string) []reply.Option
HydrateCluster(ctx context.Context, body, url string) []cluster.Option
HydrateItem(ctx context.Context, body, url string) []item.Option
HydrateThread(ctx context.Context, body string, url opt.Optional[string]) []thread.Option
HydrateReply(ctx context.Context, body string, url opt.Optional[string]) []reply.Option
HydrateCluster(ctx context.Context, body string, url opt.Optional[string]) []cluster.Option
HydrateItem(ctx context.Context, body string, url opt.Optional[string]) []item.Option
}

func Build() fx.Option {
Expand Down Expand Up @@ -51,7 +52,7 @@ func New(
}
}

func (s *service) HydrateThread(ctx context.Context, body, url string) []thread.Option {
func (s *service) HydrateThread(ctx context.Context, body string, url opt.Optional[string]) []thread.Option {
short, links, assets := s.hydrate(ctx, body, url)

return []thread.Option{
Expand All @@ -61,7 +62,7 @@ func (s *service) HydrateThread(ctx context.Context, body, url string) []thread.
}
}

func (s *service) HydrateReply(ctx context.Context, body, url string) []reply.Option {
func (s *service) HydrateReply(ctx context.Context, body string, url opt.Optional[string]) []reply.Option {
short, links, assets := s.hydrate(ctx, body, url)

return []reply.Option{
Expand All @@ -71,7 +72,7 @@ func (s *service) HydrateReply(ctx context.Context, body, url string) []reply.Op
}
}

func (s *service) HydrateCluster(ctx context.Context, body, url string) []cluster.Option {
func (s *service) HydrateCluster(ctx context.Context, body string, url opt.Optional[string]) []cluster.Option {
_, links, assets := s.hydrate(ctx, body, url)

return []cluster.Option{
Expand All @@ -80,7 +81,7 @@ func (s *service) HydrateCluster(ctx context.Context, body, url string) []cluste
}
}

func (s *service) HydrateItem(ctx context.Context, body, url string) []item.Option {
func (s *service) HydrateItem(ctx context.Context, body string, url opt.Optional[string]) []item.Option {
_, links, assets := s.hydrate(ctx, body, url)

return []item.Option{
Expand All @@ -91,10 +92,10 @@ func (s *service) HydrateItem(ctx context.Context, body, url string) []item.Opti

// hydrate takes the body and primary URL of a piece of content and fetches all
// the links and produces a short summary of the post's body text.
func (s *service) hydrate(ctx context.Context, body, url string) (string, []xid.ID, []asset.AssetID) {
func (s *service) hydrate(ctx context.Context, body string, urls opt.Optional[string]) (string, []xid.ID, []asset.AssetID) {
structured := extractor.Destructure(body)

urls := append([]string{url}, structured.Links...)
urls = append(urls, structured.Links...)

links := []xid.ID{}
assets := []asset.AssetID{}
Expand Down
5 changes: 2 additions & 3 deletions app/services/item_crud/item.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,10 @@ func (s *service) Archive(ctx context.Context, slug datagraph.ItemSlug) (*datagr

func (s *service) hydrateLink(ctx context.Context, partial Partial) (opts []item.Option) {
text, textOK := partial.Content.Get()
url, urlOK := partial.URL.Get()

if !textOK && !urlOK {
if !textOK && !partial.URL.Ok() {
return
}

return s.hydrator.HydrateItem(ctx, text, url)
return s.hydrator.HydrateItem(ctx, text, partial.URL)
}
2 changes: 1 addition & 1 deletion app/services/reply/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,5 @@ func (s *service) hydrate(ctx context.Context, partial Partial) (opts []reply.Op
return
}

return s.hydrator.HydrateReply(ctx, body, "")
return s.hydrator.HydrateReply(ctx, body, opt.NewEmpty[string]())
}
5 changes: 2 additions & 3 deletions app/services/thread/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,10 @@ func (s *service) Create(ctx context.Context,

func (s *service) hydrate(ctx context.Context, partial Partial) (opts []thread.Option) {
body, bodyOK := partial.Body.Get()
url, urlOK := partial.URL.Get()

if !bodyOK && !urlOK {
if !bodyOK && !partial.URL.Ok() {
return
}

return s.hydrator.HydrateThread(ctx, body, url)
return s.hydrator.HydrateThread(ctx, body, partial.URL)
}
4 changes: 3 additions & 1 deletion web/src/components/thread/PostView/usePostView.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { postUpdate } from "src/api/openapi/posts";
import { PostProps } from "src/api/openapi/schemas";
import { threadUpdate } from "src/api/openapi/threads";
import { threadUpdate, useThreadGet } from "src/api/openapi/threads";
import { useToast } from "src/theme/components";

import { useThreadScreenContext } from "../context/context";
Expand All @@ -15,6 +15,7 @@ export function usePostView(props: PostProps) {
setEditingContent,
} = useThreadScreenContext();
const toast = useToast();
const { mutate } = useThreadGet(thread!.slug);

const isEditing = editingPostID === props.id;
const isEditingThread = thread?.id === editingPostID;
Expand All @@ -41,6 +42,7 @@ export function usePostView(props: PostProps) {
}).then(() => toast({ title: "Post updated" }));
}

await mutate();
setEditingPostID(undefined);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,12 @@ export function useComposeForm({ initialDraft, editing }: Props) {
const doPublish = async ({ title, body, category, url }: FormShape) => {
if (editing) {
const { slug } = await threadUpdate(editing, {
title,
body,
category,
status: ThreadStatus.published,
tags: [],
url,
});
router.push(`/t/${slug}`);
} else {
Expand Down
Loading