From bc39b4c0b7f0f0454c31e3eb8527044e300e03f3 Mon Sep 17 00:00:00 2001 From: Ben Visness Date: Sat, 28 Aug 2021 12:07:45 -0500 Subject: [PATCH] Clean up TODOs --- public/js/tabs.js | 1 - public/style.css | 12 +++-- src/discord/gateway.go | 11 ++-- src/discord/history.go | 3 +- src/discord/payloads.go | 4 -- src/discord/showcase.go | 5 +- src/hmnurl/hmnurl_test.go | 2 +- src/hmnurl/urls.go | 9 ++-- src/logging/logging.go | 2 - src/models/post.go | 3 +- src/parsing/bbcode.go | 2 +- src/rawdata/scss/_content.scss | 2 +- src/rawdata/scss/_core.scss | 6 +-- src/rawdata/scss/_forms.scss | 4 ++ src/rawdata/scss/_notices.scss | 2 +- src/templates/mapping.go | 2 +- src/templates/src/blog_post.html | 3 ++ src/templates/src/editor.html | 1 - src/templates/src/forum_thread.html | 11 ++-- .../src/include/forum_post_standalone.html | 2 +- src/templates/src/include/header.html | 15 +++--- src/templates/src/layouts/base.html | 2 +- src/templates/src/project_homepage.html | 1 - src/templates/src/reject.html | 2 +- src/templates/types.go | 4 ++ src/website/blogs.go | 2 +- src/website/discord.go | 16 ++---- src/website/feed.go | 2 +- src/website/forums.go | 53 ++++++++++++++++++- src/website/imagefile_helper.go | 45 +++++++++++----- src/website/landing.go | 2 +- src/website/podcast.go | 17 +++--- src/website/requesthandling.go | 2 - src/website/routes.go | 15 +++--- src/website/user.go | 21 ++++---- 35 files changed, 172 insertions(+), 114 deletions(-) diff --git a/public/js/tabs.js b/public/js/tabs.js index 2c7b2396..2db85bdd 100644 --- a/public/js/tabs.js +++ b/public/js/tabs.js @@ -72,7 +72,6 @@ function switchTab(container, slug) { for (const tab of tabs) { const slugMatches = tab.getAttribute("data-slug") === slug; tab.classList.toggle('dn', !slugMatches); - // TODO: Also update the tab button styles if (slugMatches) { didMatch = true; diff --git a/public/style.css b/public/style.css index df32a896..c4d8f904 100644 --- a/public/style.css +++ b/public/style.css @@ -1173,7 +1173,7 @@ img, video { .br1 { border-radius: 0.125rem; } -.br2, .hmn-code, .notice { +.br2, .hmn-code { border-radius: 0.25rem; } .br3 { @@ -1209,7 +1209,7 @@ img, video { border-radius: 0; } .br1-ns { border-radius: 0.125rem; } - .br2-ns { + .br2-ns, .notice { border-radius: 0.25rem; } .br3-ns { border-radius: 0.5rem; } @@ -4581,7 +4581,7 @@ code, .code { .pa1 { padding: 0.25rem; } -.pa2, header .menu-bar .items a, header .user-options a, .tab { +.pa2, header .menu-bar .items a, .tab { padding: 0.5rem; } .pa3, header #login-popup { @@ -7675,7 +7675,8 @@ header #login-popup { .content p { -moz-text-size-adjust: auto; -webkit-text-size-adjust: auto; - text-size-adjust: auto; } + text-size-adjust: auto; + margin: 0.6rem 0; } .content .description { line-height: 1.42em; text-align: left; @@ -8232,6 +8233,9 @@ nav.timecodes { text-align: center; margin: 10px 0; } +form { + margin: 0; } + .radio, .checkbox { position: relative; display: flex; diff --git a/src/discord/gateway.go b/src/discord/gateway.go index 9e612040..f48faa43 100644 --- a/src/discord/gateway.go +++ b/src/discord/gateway.go @@ -155,12 +155,13 @@ func (bot *botInstance) Run(ctx context.Context) (err error) { for { msg, err := bot.receiveGatewayMessage(ctx) if err != nil { - // TODO: Are there other kinds of connection close events that we need to handle? Probably? if errors.Is(err, net.ErrClosed) { // If the connection is closed, that's our cue to shut down the bot. Any errors // related to the closure will have been logged elsewhere anyway. return nil } else { + // NOTE(ben): I don't know what events we might get in the future that we might + // want to handle gracefully (like above). Keep an eye out. return oops.New(err, "failed to receive message from the gateway") } } @@ -573,7 +574,7 @@ func (bot *botInstance) processEventMsg(ctx context.Context, msg *GatewayMessage return nil } -// TODO: Should this return an error? Or just log errors? +// Only return an error if we want to restart the bot. func (bot *botInstance) messageCreateOrUpdate(ctx context.Context, msg *Message) error { if msg.OriginalHasFields("author") && msg.Author.ID == config.Config.Discord.BotUserID { // Don't process your own messages @@ -583,7 +584,8 @@ func (bot *botInstance) messageCreateOrUpdate(ctx context.Context, msg *Message) if msg.ChannelID == config.Config.Discord.ShowcaseChannelID { err := bot.processShowcaseMsg(ctx, msg) if err != nil { - return oops.New(err, "failed to process showcase message") + logging.ExtractLogger(ctx).Error().Err(err).Msg("failed to process showcase message") + return nil } return nil } @@ -591,7 +593,8 @@ func (bot *botInstance) messageCreateOrUpdate(ctx context.Context, msg *Message) if msg.ChannelID == config.Config.Discord.LibraryChannelID { err := bot.processLibraryMsg(ctx, msg) if err != nil { - return oops.New(err, "failed to process library message") + logging.ExtractLogger(ctx).Error().Err(err).Msg("failed to process library message") + return nil } return nil } diff --git a/src/discord/history.go b/src/discord/history.go index 80c820b1..6b713139 100644 --- a/src/discord/history.go +++ b/src/discord/history.go @@ -137,7 +137,8 @@ func Scrape(ctx context.Context, dbConn *pgxpool.Pool, channelID string, earlies Before: before, }) if err != nil { - panic(err) // TODO + logging.Error().Err(err).Msg("failed to get messages while scraping") + return } if len(msgs) == 0 { diff --git a/src/discord/payloads.go b/src/discord/payloads.go index 4b6e3c53..33362f42 100644 --- a/src/discord/payloads.go +++ b/src/discord/payloads.go @@ -59,9 +59,6 @@ func (m *GatewayMessage) ToJSON() []byte { panic(err) } - // TODO: check if the payload is too big, either here or where we actually send - // https://discord.com/developers/docs/topics/gateway#sending-payloads - return mBytes } @@ -70,7 +67,6 @@ type Hello struct { } func HelloFromMap(m interface{}) Hello { - // TODO: This should probably have some error handling, right? return Hello{ HeartbeatIntervalMs: int(m.(map[string]interface{})["heartbeat_interval"].(float64)), } diff --git a/src/discord/showcase.go b/src/discord/showcase.go index 8fdc96c1..fa7b267b 100644 --- a/src/discord/showcase.go +++ b/src/discord/showcase.go @@ -404,6 +404,8 @@ func saveAttachment( return iDiscordAttachment.(*models.DiscordMessageAttachment), nil } +// Saves an embed from Discord. NOTE: This is _not_ idempotent, so only call it +// if you do not have any embeds saved for this message yet. func saveEmbed( ctx context.Context, tx db.ConnOrTx, @@ -411,9 +413,6 @@ func saveEmbed( hmnUserID int, discordMessageID string, ) (*models.DiscordMessageEmbed, error) { - // TODO: Does this need to be idempotent? Embeds don't have IDs... - // Maybe Discord will never actually send us the same embed twice? - isOkImageType := func(contentType string) bool { return strings.HasPrefix(contentType, "image/") } diff --git a/src/hmnurl/hmnurl_test.go b/src/hmnurl/hmnurl_test.go index ff47260d..24040e53 100644 --- a/src/hmnurl/hmnurl_test.go +++ b/src/hmnurl/hmnurl_test.go @@ -308,7 +308,7 @@ func TestPublic(t *testing.T) { } func TestForumMarkRead(t *testing.T) { - AssertRegexMatch(t, BuildForumMarkRead(5), RegexForumMarkRead, map[string]string{"sfid": "5"}) + AssertRegexMatch(t, BuildForumMarkRead(c.CurrentProject.Slug, 5), RegexForumMarkRead, map[string]string{"sfid": "5"}) } func AssertSubdomain(t *testing.T, fullUrl string, expectedSubdomain string) { diff --git a/src/hmnurl/urls.go b/src/hmnurl/urls.go index ea21fa5b..f919677c 100644 --- a/src/hmnurl/urls.go +++ b/src/hmnurl/urls.go @@ -354,8 +354,8 @@ func BuildPodcastEpisodeFile(projectSlug string, filename string) string { * Forums */ -// TODO(asaf): This also matches urls generated by BuildForumThread (/t/ is identified as a subforum, and the threadid as a page) -// This shouldn't be a problem since we will match Thread before Subforum in the router, but should we enforce it here? +// NOTE(asaf): This also matches urls generated by BuildForumThread (/t/ is identified as a subforum, and the threadid as a page) +// Make sure to match Thread before Subforum in the router. var RegexForum = regexp.MustCompile(`^/forums(/(?P[^\d/]+(/[^\d]+)*))?(/(?P\d+))?$`) func BuildForum(projectSlug string, subforums []string, page int) string { @@ -433,7 +433,6 @@ func BuildForumPostEdit(projectSlug string, subforums []string, threadId int, po var RegexForumPostReply = regexp.MustCompile(`^/forums(/(?P[^\d/]+(/[^\d]+)*))?/t/(?P\d+)/p/(?P\d+)/reply$`) -// TODO: It's kinda weird that we have "replies" to a specific post. That's not how the data works. I guess this just affects what you see as the "post you're replying to" on the post composer page? func BuildForumPostReply(projectSlug string, subforums []string, threadId int, postId int) string { defer CatchPanic() builder := buildForumPostPath(subforums, threadId, postId) @@ -689,7 +688,7 @@ func BuildUserFile(filepath string) string { var RegexForumMarkRead = regexp.MustCompile(`^/markread/(?P\d+)$`) // NOTE(asaf): subforumId == 0 means ALL SUBFORUMS -func BuildForumMarkRead(subforumId int) string { +func BuildForumMarkRead(projectSlug string, subforumId int) string { defer CatchPanic() if subforumId < 0 { panic(oops.New(nil, "Invalid subforum ID (%d), must be >= 0", subforumId)) @@ -699,7 +698,7 @@ func BuildForumMarkRead(subforumId int) string { builder.WriteString("/markread/") builder.WriteString(strconv.Itoa(subforumId)) - return Url(builder.String(), nil) + return ProjectUrl(builder.String(), nil, projectSlug) } var RegexCatchAll = regexp.MustCompile("") diff --git a/src/logging/logging.go b/src/logging/logging.go index 243765fc..10c07595 100644 --- a/src/logging/logging.go +++ b/src/logging/logging.go @@ -96,8 +96,6 @@ func NewPrettyZerologWriter() *PrettyZerologWriter { } func (w *PrettyZerologWriter) Write(p []byte) (int, error) { - // TODO: panic recovery so we log _something_ - var fields map[string]interface{} err := json.Unmarshal(p, &fields) if err != nil { diff --git a/src/models/post.go b/src/models/post.go index cb56de08..0799f785 100644 --- a/src/models/post.go +++ b/src/models/post.go @@ -8,10 +8,9 @@ import ( type Post struct { ID int `db:"id"` - // TODO: Document each of these AuthorID *int `db:"author_id"` ThreadID int `db:"thread_id"` - CurrentID int `db:"current_id"` + CurrentID int `db:"current_id"` // The id of the current PostVersion ProjectID int `db:"project_id"` ThreadType ThreadType `db:"thread_type"` diff --git a/src/parsing/bbcode.go b/src/parsing/bbcode.go index 30b2b873..ed986858 100644 --- a/src/parsing/bbcode.go +++ b/src/parsing/bbcode.go @@ -21,7 +21,7 @@ import ( "github.com/yuin/goldmark/util" ) -var BBCodePriority = 1 // TODO: This is maybe too high a priority? +var BBCodePriority = 1 var reOpenTag = regexp.MustCompile(`^\[\s*(?P[a-zA-Z0-9]+)`) var reTag = regexp.MustCompile(`\[\s*(?P[a-zA-Z0-9]+)|\[\s*\/\s*(?P[a-zA-Z0-9]+)\s*\]`) diff --git a/src/rawdata/scss/_content.scss b/src/rawdata/scss/_content.scss index b23eb050..3762b94e 100644 --- a/src/rawdata/scss/_content.scss +++ b/src/rawdata/scss/_content.scss @@ -60,4 +60,4 @@ pre { @extend .br2; padding: 0.7em; -} \ No newline at end of file +} diff --git a/src/rawdata/scss/_core.scss b/src/rawdata/scss/_core.scss index b97cebb3..72070ea2 100644 --- a/src/rawdata/scss/_core.scss +++ b/src/rawdata/scss/_core.scss @@ -505,10 +505,6 @@ header { .user-options { position: relative; - - a { - @extend .pa2; - } } .login, .register { @@ -571,6 +567,8 @@ footer { -moz-text-size-adjust:auto; -webkit-text-size-adjust:auto; text-size-adjust:auto; + + margin: 0.6rem 0; } .description { diff --git a/src/rawdata/scss/_forms.scss b/src/rawdata/scss/_forms.scss index 2b794da7..555e670f 100644 --- a/src/rawdata/scss/_forms.scss +++ b/src/rawdata/scss/_forms.scss @@ -1,3 +1,7 @@ +form { + margin: 0; +} + .radio, .checkbox { $size: 1.3rem; $margin: 0.5rem; diff --git a/src/rawdata/scss/_notices.scss b/src/rawdata/scss/_notices.scss index e79e9a9a..6b4efdf8 100644 --- a/src/rawdata/scss/_notices.scss +++ b/src/rawdata/scss/_notices.scss @@ -1,7 +1,7 @@ .notice { @include usevar(color, notice-text-color); @extend .ph3, .pv2; - @extend .br2; + @extend .br2-ns; a { @include usevar(color, notice-text-color); diff --git a/src/templates/mapping.go b/src/templates/mapping.go index 36e4ee73..0e998ca0 100644 --- a/src/templates/mapping.go +++ b/src/templates/mapping.go @@ -273,7 +273,7 @@ func TimelineItemsToJSON(items []TimelineItem) string { builder.WriteString(`",`) builder.WriteString(`"owner_name":"`) - builder.WriteString(item.OwnerName) // TODO: Do we need to do escaping on these other string fields too? Feels like someone could use this for XSS. + builder.WriteString(item.OwnerName) builder.WriteString(`",`) builder.WriteString(`"owner_avatar":"`) diff --git a/src/templates/src/blog_post.html b/src/templates/src/blog_post.html index b5f2f4c5..7866c86f 100644 --- a/src/templates/src/blog_post.html +++ b/src/templates/src/blog_post.html @@ -93,6 +93,9 @@ {{ end }} ↪  {{ end }} + + #{{ .ID }} + {{ end }} diff --git a/src/templates/src/editor.html b/src/templates/src/editor.html index 6acb6496..5d21256b 100644 --- a/src/templates/src/editor.html +++ b/src/templates/src/editor.html @@ -3,7 +3,6 @@ {{ define "extrahead" }} {{/* TODO: These are no longer useful? */}} - diff --git a/src/templates/src/forum_thread.html b/src/templates/src/forum_thread.html index b0147b8c..69aece7b 100644 --- a/src/templates/src/forum_thread.html +++ b/src/templates/src/forum_thread.html @@ -34,13 +34,12 @@
- {{/* TODO: Aggregate user data
- {{ post.author.posts }} posts - {% if post.author.public_projects.values|length > 0 %} - / {{ post.author.public_projects.values|length }} project{%if post.author.public_projects.values|length > 1 %}s{% endif %} - {% endif %} -
*/}} + {{ .AuthorNumPosts }} posts + {{ if gt .AuthorNumProjects 0 }} + / {{ .AuthorNumProjects }} project{{ if gt .AuthorNumProjects 1 }}s{{ end }} + {{ end }} +
{{ if .Author.IsStaff }} diff --git a/src/templates/src/include/forum_post_standalone.html b/src/templates/src/include/forum_post_standalone.html index 72def488..946074fa 100644 --- a/src/templates/src/include/forum_post_standalone.html +++ b/src/templates/src/include/forum_post_standalone.html @@ -6,7 +6,7 @@
- {{ .Author.Username }} {{/* TODO: Text scale stuff? Seems unnecessary. */}} + {{ .Author.Username }}
{{ if .Author.IsStaff }} diff --git a/src/templates/src/include/header.html b/src/templates/src/include/header.html index 2aa2cfa1..34e8fe56 100644 --- a/src/templates/src/include/header.html +++ b/src/templates/src/include/header.html @@ -2,13 +2,16 @@
{{ if .User }} {{ if .User.IsStaff }} - Admin + Admin {{ end }} - {{ .User.Username }} - Log Out + + Log Out {{ else }} - Register - + Register +
@@ -58,7 +61,7 @@
- +
diff --git a/src/templates/src/layouts/base.html b/src/templates/src/layouts/base.html index f2fc77d0..6cc67dab 100644 --- a/src/templates/src/layouts/base.html +++ b/src/templates/src/layouts/base.html @@ -12,7 +12,7 @@ {{ end }} {{ end }} {{ if .Title }} - {{ .Title }} | Handmade Network {{/* TODO: Some parts of the site replace "Handmade Network" with other things like "4coder Forums". */}} + {{ .Title }} | Handmade Network {{ else }} Handmade Network {{ end }} diff --git a/src/templates/src/project_homepage.html b/src/templates/src/project_homepage.html index eb3a9aae..f81a823e 100644 --- a/src/templates/src/project_homepage.html +++ b/src/templates/src/project_homepage.html @@ -47,7 +47,6 @@
{{ end }} - {{/* TODO(asaf): Add timeline items for project */}}
diff --git a/src/templates/src/reject.html b/src/templates/src/reject.html index 46847621..08cb7610 100644 --- a/src/templates/src/reject.html +++ b/src/templates/src/reject.html @@ -1,7 +1,7 @@ {{ template "base.html" . }} {{ define "content" }} -
+

Hi there, {{ if .User }}{{ .User.Name }}{{ else }}visitor{{ end }}!

diff --git a/src/templates/types.go b/src/templates/types.go index 7d4f1ca7..ed29186d 100644 --- a/src/templates/types.go +++ b/src/templates/types.go @@ -39,6 +39,7 @@ func (bd *BaseData) AddImmediateNotice(class, content string) { type Header struct { AdminUrl string + UserProfileUrl string UserSettingsUrl string LoginActionUrl string LogoutActionUrl string @@ -90,6 +91,9 @@ type Post struct { Content template.HTML PostDate time.Time + AuthorNumPosts int + AuthorNumProjects int + Editor *User EditDate time.Time EditReason string diff --git a/src/website/blogs.go b/src/website/blogs.go index 8b264098..c6e64812 100644 --- a/src/website/blogs.go +++ b/src/website/blogs.go @@ -204,7 +204,7 @@ func BlogPostRedirectToThread(c *RequestContext) ResponseData { thread := FetchThread(c.Context(), c.Conn, cd.ThreadID) - threadUrl := hmnurl.BuildBlogThread(c.CurrentProject.Slug, cd.ThreadID, thread.Title) + threadUrl := hmnurl.BuildBlogThreadWithPostHash(c.CurrentProject.Slug, cd.ThreadID, thread.Title, cd.PostID) return c.Redirect(threadUrl, http.StatusFound) } diff --git a/src/website/discord.go b/src/website/discord.go index b03cd212..9a75fbaa 100644 --- a/src/website/discord.go +++ b/src/website/discord.go @@ -5,7 +5,6 @@ import ( "net/http" "time" - "git.handmade.network/hmn/hmn/src/auth" "git.handmade.network/hmn/hmn/src/config" "git.handmade.network/hmn/hmn/src/db" "git.handmade.network/hmn/hmn/src/discord" @@ -22,25 +21,16 @@ func DiscordOAuthCallback(c *RequestContext) ResponseData { if state != c.CurrentSession.CSRFToken { // CSRF'd!!!! - // TODO(compression): Should this and the CSRF middleware be pulled out to - // a separate function? - c.Logger.Warn().Str("userId", c.CurrentUser.Username).Msg("user failed Discord OAuth state validation - potential attack?") - err := auth.DeleteSession(c.Context(), c.Conn, c.CurrentSession.ID) - if err != nil { - c.Logger.Error().Err(err).Msg("failed to delete session on Discord OAuth state failure") - } - res := c.Redirect("/", http.StatusSeeOther) - res.SetCookie(auth.DeleteSessionCookie) + logoutUser(c, &res) return res } - // Check for error values and redirect back to ???? + // Check for error values and redirect back to user settings if errCode := query.Get("error"); errCode != "" { - // TODO: actually handle these errors if errCode == "access_denied" { // This occurs when the user cancels. Just go back to the profile page. return c.Redirect(hmnurl.BuildUserSettings("discord"), http.StatusSeeOther) @@ -49,7 +39,7 @@ func DiscordOAuthCallback(c *RequestContext) ResponseData { } } - // Do the actual token exchange and redirect back to ???? + // Do the actual token exchange code := query.Get("code") res, err := discord.ExchangeOAuthCode(c.Context(), code, hmnurl.BuildDiscordOAuthCallback()) if err != nil { diff --git a/src/website/feed.go b/src/website/feed.go index aec49462..6ec3ae05 100644 --- a/src/website/feed.go +++ b/src/website/feed.go @@ -98,7 +98,7 @@ func Feed(c *RequestContext) ResponseData { BaseData: baseData, AtomFeedUrl: hmnurl.BuildAtomFeed(), - MarkAllReadUrl: hmnurl.BuildForumMarkRead(0), + MarkAllReadUrl: hmnurl.BuildForumMarkRead(c.CurrentProject.Slug, 0), Posts: posts, Pagination: pagination, }, c.Perf) diff --git a/src/website/forums.go b/src/website/forums.go index a22f11ec..15ce459a 100644 --- a/src/website/forums.go +++ b/src/website/forums.go @@ -1,6 +1,7 @@ package website import ( + "context" "fmt" "net/http" "strconv" @@ -9,6 +10,7 @@ import ( "git.handmade.network/hmn/hmn/src/db" "git.handmade.network/hmn/hmn/src/hmnurl" + "git.handmade.network/hmn/hmn/src/logging" "git.handmade.network/hmn/hmn/src/models" "git.handmade.network/hmn/hmn/src/oops" "git.handmade.network/hmn/hmn/src/templates" @@ -283,7 +285,7 @@ func Forum(c *RequestContext) ResponseData { res.MustWriteTemplate("forum.html", forumData{ BaseData: baseData, NewThreadUrl: hmnurl.BuildForumNewThread(c.CurrentProject.Slug, currentSubforumSlugs, false), - MarkReadUrl: hmnurl.BuildForumMarkRead(cd.SubforumID), + MarkReadUrl: hmnurl.BuildForumMarkRead(c.CurrentProject.Slug, cd.SubforumID), Threads: threads, Pagination: templates.Pagination{ Current: page, @@ -470,6 +472,7 @@ func ForumThread(c *RequestContext) ResponseData { ) c.Perf.EndBlock() + c.Perf.StartBlock("TEMPLATE", "Create template posts") var posts []templates.Post for _, p := range postsAndStuff { post := templates.PostToTemplate(&p.Post, p.Author, c.Theme) @@ -482,8 +485,11 @@ func ForumThread(c *RequestContext) ResponseData { post.ReplyPost = &reply } + addAuthorCountsToPost(c.Context(), c.Conn, &post) + posts = append(posts, post) } + c.Perf.EndBlock() // Update thread last read info if c.CurrentUser != nil { @@ -774,9 +780,11 @@ func ForumPostEditSubmit(c *RequestContext) ResponseData { defer tx.Rollback(c.Context()) c.Req.ParseForm() - // TODO(ben): Validation unparsed := c.Req.Form.Get("body") editReason := c.Req.Form.Get("editreason") + if unparsed == "" { + return RejectRequest(c, "You must provide a body for your post.") + } CreatePostVersion(c.Context(), tx, cd.PostID, unparsed, c.Req.Host, editReason, &c.CurrentUser.ID) @@ -999,3 +1007,44 @@ func addForumUrlsToPost(p *templates.Post, projectSlug string, subforums []strin p.EditUrl = hmnurl.BuildForumPostEdit(projectSlug, subforums, threadId, postId) p.ReplyUrl = hmnurl.BuildForumPostReply(projectSlug, subforums, threadId, postId) } + +func addAuthorCountsToPost(ctx context.Context, conn db.ConnOrTx, p *templates.Post) { + numPosts, err := db.QueryInt(ctx, conn, + ` + SELECT COUNT(*) + FROM + handmade_post AS post + JOIN handmade_project AS project ON post.project_id = project.id + WHERE + post.author_id = $1 + AND NOT post.deleted + AND project.lifecycle = ANY ($2) + `, + p.Author.ID, + models.VisibleProjectLifecycles, + ) + if err != nil { + logging.ExtractLogger(ctx).Warn().Err(err).Msg("failed to get count of user posts") + } else { + p.AuthorNumPosts = numPosts + } + + numProjects, err := db.QueryInt(ctx, conn, + ` + SELECT COUNT(*) + FROM + handmade_project AS project + JOIN handmade_user_projects AS uproj ON uproj.project_id = project.id + WHERE + project.lifecycle = ANY ($1) + AND uproj.user_id = $2 + `, + models.VisibleProjectLifecycles, + p.Author.ID, + ) + if err != nil { + logging.ExtractLogger(ctx).Warn().Err(err).Msg("failed to get count of user projects") + } else { + p.AuthorNumProjects = numProjects + } +} diff --git a/src/website/imagefile_helper.go b/src/website/imagefile_helper.go index 44b9bd63..65d07d6b 100644 --- a/src/website/imagefile_helper.go +++ b/src/website/imagefile_helper.go @@ -14,38 +14,49 @@ import ( "git.handmade.network/hmn/hmn/src/oops" ) -// If a helper method returns this, you should call RejectRequest with the value. -type RejectRequestError error +type SaveImageFileResult struct { + ImageFileID int + ValidationError string + FatalError error +} /* Reads an image file from form data and saves it to the filesystem and the database. -If the file doesn't exist, this does nothing. +If the file doesn't exist, this does nothing and returns 0 for the image file id. NOTE(ben): Someday we should replace this with the asset system. */ -func SaveImageFile(c *RequestContext, dbConn db.ConnOrTx, fileFieldName string, maxSize int64, filepath string) (imageFileId int, err error) { +func SaveImageFile(c *RequestContext, dbConn db.ConnOrTx, fileFieldName string, maxSize int64, filepath string) SaveImageFileResult { img, header, err := c.Req.FormFile(fileFieldName) filename := "" width := 0 height := 0 if err != nil && !errors.Is(err, http.ErrMissingFile) { - return 0, oops.New(err, "failed to read uploaded file") + return SaveImageFileResult{ + FatalError: oops.New(err, "failed to read uploaded file"), + } } if header != nil { if header.Size > maxSize { - return 0, RejectRequestError(fmt.Errorf("Image filesize too big. Max size: %d bytes", maxSize)) + return SaveImageFileResult{ + ValidationError: fmt.Sprintf("Image filesize too big. Max size: %d bytes", maxSize), + } } else { c.Perf.StartBlock("IMAGE", "Decoding image") config, format, err := image.DecodeConfig(img) c.Perf.EndBlock() if err != nil { - return 0, RejectRequestError(errors.New("Image type not supported")) + return SaveImageFileResult{ + ValidationError: "Image type not supported", + } } width = config.Width height = config.Height if width == 0 || height == 0 { - return 0, RejectRequestError(errors.New("Image has zero size")) + return SaveImageFileResult{ + ValidationError: "Image has zero size", + } } filename = fmt.Sprintf("%s.%s", filepath, format) @@ -53,12 +64,16 @@ func SaveImageFile(c *RequestContext, dbConn db.ConnOrTx, fileFieldName string, c.Perf.StartBlock("IMAGE", "Writing image file") file, err := os.Create(storageFilename) if err != nil { - return 0, oops.New(err, "Failed to create local image file") + return SaveImageFileResult{ + FatalError: oops.New(err, "Failed to create local image file"), + } } img.Seek(0, io.SeekStart) _, err = io.Copy(file, img) if err != nil { - return 0, oops.New(err, "Failed to write image to file") + return SaveImageFileResult{ + FatalError: oops.New(err, "Failed to write image to file"), + } } file.Close() img.Close() @@ -83,11 +98,15 @@ func SaveImageFile(c *RequestContext, dbConn db.ConnOrTx, fileFieldName string, filename, header.Size, hex.EncodeToString(sha1sum), false, width, height, ).Scan(&imageId) if err != nil { - return 0, oops.New(err, "Failed to insert image file row") + return SaveImageFileResult{ + FatalError: oops.New(err, "Failed to insert image file row"), + } } - return imageId, nil + return SaveImageFileResult{ + ImageFileID: imageId, + } } - return 0, nil + return SaveImageFileResult{} } diff --git a/src/website/landing.go b/src/website/landing.go index 09952e73..0c02aae4 100644 --- a/src/website/landing.go +++ b/src/website/landing.go @@ -334,7 +334,7 @@ func Index(c *RequestContext) ResponseData { Url: hmnurl.BuildBlogThread(models.HMNProjectSlug, newsPostResult.Thread.ID, newsPostResult.Thread.Title), User: templates.UserToTemplate(&newsPostResult.User, c.Theme), Date: newsPostResult.Post.PostDate, - Unread: true, // TODO + Unread: true, Content: template.HTML(newsPostResult.PostVersion.TextParsed), }, PostColumns: cols, diff --git a/src/website/podcast.go b/src/website/podcast.go index 3f820409..af280622 100644 --- a/src/website/podcast.go +++ b/src/website/podcast.go @@ -147,17 +147,14 @@ func PodcastEditSubmit(c *RequestContext) ResponseData { } defer tx.Rollback(c.Context()) - imageId, err := SaveImageFile(c, tx, "podcast_image", maxFileSize, fmt.Sprintf("podcast/%s/logo%d", c.CurrentProject.Slug, time.Now().UTC().Unix())) - if err != nil { - var rejectErr RejectRequestError - if errors.As(err, &rejectErr) { - return RejectRequest(c, rejectErr.Error()) - } else { - return c.ErrorResponse(http.StatusInternalServerError, oops.New(err, "Failed to save podcast image")) - } + imageSaveResult := SaveImageFile(c, tx, "podcast_image", maxFileSize, fmt.Sprintf("podcast/%s/logo%d", c.CurrentProject.Slug, time.Now().UTC().Unix())) + if imageSaveResult.ValidationError != "" { + return RejectRequest(c, imageSaveResult.ValidationError) + } else if imageSaveResult.FatalError != nil { + return c.ErrorResponse(http.StatusInternalServerError, oops.New(imageSaveResult.FatalError, "Failed to save podcast image")) } - if imageId != 0 { + if imageSaveResult.ImageFileID != 0 { _, err = tx.Exec(c.Context(), ` UPDATE handmade_podcast @@ -169,7 +166,7 @@ func PodcastEditSubmit(c *RequestContext) ResponseData { `, title, description, - imageId, + imageSaveResult.ImageFileID, podcastResult.Podcast.ID, ) if err != nil { diff --git a/src/website/requesthandling.go b/src/website/requesthandling.go index 57e3e49a..0841571b 100644 --- a/src/website/requesthandling.go +++ b/src/website/requesthandling.go @@ -107,8 +107,6 @@ func (r *Router) ServeHTTP(rw http.ResponseWriter, req *http.Request) { return } - // TODO(asaf): Replace this with a 404 handler? Isn't this going to crash the server? - // We're doing panic recovery in doRequest, but not here. I don't think we should have this line in production. panic(fmt.Sprintf("Path '%s' did not match any routes! Make sure to register a wildcard route to act as a 404.", path)) } diff --git a/src/website/routes.go b/src/website/routes.go index ebea9c5b..dfaa10e8 100644 --- a/src/website/routes.go +++ b/src/website/routes.go @@ -112,13 +112,8 @@ func NewWebsiteRoutes(longRequestContext context.Context, conn *pgxpool.Pool, pe if csrfToken != c.CurrentSession.CSRFToken { c.Logger.Warn().Str("userId", c.CurrentUser.Username).Msg("user failed CSRF validation - potential attack?") - err := auth.DeleteSession(c.Context(), c.Conn, c.CurrentSession.ID) - if err != nil { - c.Logger.Error().Err(err).Msg("failed to delete session on CSRF failure") - } - res := c.Redirect("/", http.StatusSeeOther) - res.SetCookie(auth.DeleteSessionCookie) + logoutUser(c, &res) return res } @@ -275,7 +270,7 @@ func getBaseData(c *RequestContext) templates.BaseData { episodeGuideUrl = hmnurl.BuildEpisodeList(c.CurrentProject.Slug, defaultTopic) } - return templates.BaseData{ + baseData := templates.BaseData{ Theme: c.Theme, CurrentUrl: c.FullUrl(), @@ -322,6 +317,12 @@ func getBaseData(c *RequestContext) templates.BaseData { SitemapUrl: hmnurl.BuildSiteMap(), }, } + + if c.CurrentUser != nil { + baseData.Header.UserProfileUrl = hmnurl.BuildUserProfile(c.CurrentUser.Username) + } + + return baseData } func buildDefaultOpenGraphItems(project *models.Project) []templates.OpenGraphItem { diff --git a/src/website/user.go b/src/website/user.go index 01d7b550..2ec35d1a 100644 --- a/src/website/user.go +++ b/src/website/user.go @@ -454,27 +454,24 @@ func UserSettingsSave(c *RequestContext) ResponseData { } // Update avatar - _, err = SaveImageFile(c, tx, "avatar", 1*1024*1024, fmt.Sprintf("members/avatars/%s", c.CurrentUser.Username)) - if err != nil { - var rejectErr RejectRequestError - if errors.As(err, &rejectErr) { - return RejectRequest(c, rejectErr.Error()) - } else { - return c.ErrorResponse(http.StatusInternalServerError, oops.New(err, "failed to save new avatar")) - } + imageSaveResult := SaveImageFile(c, tx, "avatar", 1*1024*1024, fmt.Sprintf("members/avatars/%s", c.CurrentUser.Username)) + if imageSaveResult.ValidationError != "" { + return RejectRequest(c, imageSaveResult.ValidationError) + } else if imageSaveResult.FatalError != nil { + return c.ErrorResponse(http.StatusInternalServerError, oops.New(imageSaveResult.FatalError, "failed to save new avatar")) } - // TODO: Success message - err = tx.Commit(c.Context()) if err != nil { return c.ErrorResponse(http.StatusInternalServerError, oops.New(err, "failed to save user settings")) } - return c.Redirect(hmnurl.BuildUserSettings(""), http.StatusSeeOther) + res := c.Redirect(hmnurl.BuildUserSettings(""), http.StatusSeeOther) + res.AddFutureNotice("success", "User profile updated.") + + return res } -// TODO: Rework this to use that RejectRequestError thing func updatePassword(c *RequestContext, tx pgx.Tx, old, new, confirm string) *ResponseData { if new != confirm { res := RejectRequest(c, "Your password and password confirmation did not match.")