Clean up TODOs

This commit is contained in:
Ben Visness 2021-08-28 12:07:45 -05:00
parent 57d4216d2d
commit bc39b4c0b7
35 changed files with 172 additions and 114 deletions

View File

@ -72,7 +72,6 @@ function switchTab(container, slug) {
for (const tab of tabs) { for (const tab of tabs) {
const slugMatches = tab.getAttribute("data-slug") === slug; const slugMatches = tab.getAttribute("data-slug") === slug;
tab.classList.toggle('dn', !slugMatches); tab.classList.toggle('dn', !slugMatches);
// TODO: Also update the tab button styles
if (slugMatches) { if (slugMatches) {
didMatch = true; didMatch = true;

View File

@ -1173,7 +1173,7 @@ img, video {
.br1 { .br1 {
border-radius: 0.125rem; } border-radius: 0.125rem; }
.br2, .hmn-code, .notice { .br2, .hmn-code {
border-radius: 0.25rem; } border-radius: 0.25rem; }
.br3 { .br3 {
@ -1209,7 +1209,7 @@ img, video {
border-radius: 0; } border-radius: 0; }
.br1-ns { .br1-ns {
border-radius: 0.125rem; } border-radius: 0.125rem; }
.br2-ns { .br2-ns, .notice {
border-radius: 0.25rem; } border-radius: 0.25rem; }
.br3-ns { .br3-ns {
border-radius: 0.5rem; } border-radius: 0.5rem; }
@ -4581,7 +4581,7 @@ code, .code {
.pa1 { .pa1 {
padding: 0.25rem; } padding: 0.25rem; }
.pa2, header .menu-bar .items a, header .user-options a, .tab { .pa2, header .menu-bar .items a, .tab {
padding: 0.5rem; } padding: 0.5rem; }
.pa3, header #login-popup { .pa3, header #login-popup {
@ -7675,7 +7675,8 @@ header #login-popup {
.content p { .content p {
-moz-text-size-adjust: auto; -moz-text-size-adjust: auto;
-webkit-text-size-adjust: auto; -webkit-text-size-adjust: auto;
text-size-adjust: auto; } text-size-adjust: auto;
margin: 0.6rem 0; }
.content .description { .content .description {
line-height: 1.42em; line-height: 1.42em;
text-align: left; text-align: left;
@ -8232,6 +8233,9 @@ nav.timecodes {
text-align: center; text-align: center;
margin: 10px 0; } margin: 10px 0; }
form {
margin: 0; }
.radio, .checkbox { .radio, .checkbox {
position: relative; position: relative;
display: flex; display: flex;

View File

@ -155,12 +155,13 @@ func (bot *botInstance) Run(ctx context.Context) (err error) {
for { for {
msg, err := bot.receiveGatewayMessage(ctx) msg, err := bot.receiveGatewayMessage(ctx)
if err != nil { 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 errors.Is(err, net.ErrClosed) {
// If the connection is closed, that's our cue to shut down the bot. Any errors // 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. // related to the closure will have been logged elsewhere anyway.
return nil return nil
} else { } 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") 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 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 { func (bot *botInstance) messageCreateOrUpdate(ctx context.Context, msg *Message) error {
if msg.OriginalHasFields("author") && msg.Author.ID == config.Config.Discord.BotUserID { if msg.OriginalHasFields("author") && msg.Author.ID == config.Config.Discord.BotUserID {
// Don't process your own messages // 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 { if msg.ChannelID == config.Config.Discord.ShowcaseChannelID {
err := bot.processShowcaseMsg(ctx, msg) err := bot.processShowcaseMsg(ctx, msg)
if err != nil { 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 return nil
} }
@ -591,7 +593,8 @@ func (bot *botInstance) messageCreateOrUpdate(ctx context.Context, msg *Message)
if msg.ChannelID == config.Config.Discord.LibraryChannelID { if msg.ChannelID == config.Config.Discord.LibraryChannelID {
err := bot.processLibraryMsg(ctx, msg) err := bot.processLibraryMsg(ctx, msg)
if err != nil { 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 return nil
} }

View File

@ -137,7 +137,8 @@ func Scrape(ctx context.Context, dbConn *pgxpool.Pool, channelID string, earlies
Before: before, Before: before,
}) })
if err != nil { if err != nil {
panic(err) // TODO logging.Error().Err(err).Msg("failed to get messages while scraping")
return
} }
if len(msgs) == 0 { if len(msgs) == 0 {

View File

@ -59,9 +59,6 @@ func (m *GatewayMessage) ToJSON() []byte {
panic(err) 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 return mBytes
} }
@ -70,7 +67,6 @@ type Hello struct {
} }
func HelloFromMap(m interface{}) Hello { func HelloFromMap(m interface{}) Hello {
// TODO: This should probably have some error handling, right?
return Hello{ return Hello{
HeartbeatIntervalMs: int(m.(map[string]interface{})["heartbeat_interval"].(float64)), HeartbeatIntervalMs: int(m.(map[string]interface{})["heartbeat_interval"].(float64)),
} }

View File

@ -404,6 +404,8 @@ func saveAttachment(
return iDiscordAttachment.(*models.DiscordMessageAttachment), nil 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( func saveEmbed(
ctx context.Context, ctx context.Context,
tx db.ConnOrTx, tx db.ConnOrTx,
@ -411,9 +413,6 @@ func saveEmbed(
hmnUserID int, hmnUserID int,
discordMessageID string, discordMessageID string,
) (*models.DiscordMessageEmbed, error) { ) (*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 { isOkImageType := func(contentType string) bool {
return strings.HasPrefix(contentType, "image/") return strings.HasPrefix(contentType, "image/")
} }

View File

@ -308,7 +308,7 @@ func TestPublic(t *testing.T) {
} }
func TestForumMarkRead(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) { func AssertSubdomain(t *testing.T, fullUrl string, expectedSubdomain string) {

View File

@ -354,8 +354,8 @@ func BuildPodcastEpisodeFile(projectSlug string, filename string) string {
* Forums * Forums
*/ */
// TODO(asaf): This also matches urls generated by BuildForumThread (/t/ is identified as a subforum, and the threadid as a page) // NOTE(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? // Make sure to match Thread before Subforum in the router.
var RegexForum = regexp.MustCompile(`^/forums(/(?P<subforums>[^\d/]+(/[^\d]+)*))?(/(?P<page>\d+))?$`) var RegexForum = regexp.MustCompile(`^/forums(/(?P<subforums>[^\d/]+(/[^\d]+)*))?(/(?P<page>\d+))?$`)
func BuildForum(projectSlug string, subforums []string, page int) string { 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<subforums>[^\d/]+(/[^\d]+)*))?/t/(?P<threadid>\d+)/p/(?P<postid>\d+)/reply$`) var RegexForumPostReply = regexp.MustCompile(`^/forums(/(?P<subforums>[^\d/]+(/[^\d]+)*))?/t/(?P<threadid>\d+)/p/(?P<postid>\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 { func BuildForumPostReply(projectSlug string, subforums []string, threadId int, postId int) string {
defer CatchPanic() defer CatchPanic()
builder := buildForumPostPath(subforums, threadId, postId) builder := buildForumPostPath(subforums, threadId, postId)
@ -689,7 +688,7 @@ func BuildUserFile(filepath string) string {
var RegexForumMarkRead = regexp.MustCompile(`^/markread/(?P<sfid>\d+)$`) var RegexForumMarkRead = regexp.MustCompile(`^/markread/(?P<sfid>\d+)$`)
// NOTE(asaf): subforumId == 0 means ALL SUBFORUMS // NOTE(asaf): subforumId == 0 means ALL SUBFORUMS
func BuildForumMarkRead(subforumId int) string { func BuildForumMarkRead(projectSlug string, subforumId int) string {
defer CatchPanic() defer CatchPanic()
if subforumId < 0 { if subforumId < 0 {
panic(oops.New(nil, "Invalid subforum ID (%d), must be >= 0", subforumId)) 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("/markread/")
builder.WriteString(strconv.Itoa(subforumId)) builder.WriteString(strconv.Itoa(subforumId))
return Url(builder.String(), nil) return ProjectUrl(builder.String(), nil, projectSlug)
} }
var RegexCatchAll = regexp.MustCompile("") var RegexCatchAll = regexp.MustCompile("")

View File

@ -96,8 +96,6 @@ func NewPrettyZerologWriter() *PrettyZerologWriter {
} }
func (w *PrettyZerologWriter) Write(p []byte) (int, error) { func (w *PrettyZerologWriter) Write(p []byte) (int, error) {
// TODO: panic recovery so we log _something_
var fields map[string]interface{} var fields map[string]interface{}
err := json.Unmarshal(p, &fields) err := json.Unmarshal(p, &fields)
if err != nil { if err != nil {

View File

@ -8,10 +8,9 @@ import (
type Post struct { type Post struct {
ID int `db:"id"` ID int `db:"id"`
// TODO: Document each of these
AuthorID *int `db:"author_id"` AuthorID *int `db:"author_id"`
ThreadID int `db:"thread_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"` ProjectID int `db:"project_id"`
ThreadType ThreadType `db:"thread_type"` ThreadType ThreadType `db:"thread_type"`

View File

@ -21,7 +21,7 @@ import (
"github.com/yuin/goldmark/util" "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<name>[a-zA-Z0-9]+)`) var reOpenTag = regexp.MustCompile(`^\[\s*(?P<name>[a-zA-Z0-9]+)`)
var reTag = regexp.MustCompile(`\[\s*(?P<opentagname>[a-zA-Z0-9]+)|\[\s*\/\s*(?P<closetagname>[a-zA-Z0-9]+)\s*\]`) var reTag = regexp.MustCompile(`\[\s*(?P<opentagname>[a-zA-Z0-9]+)|\[\s*\/\s*(?P<closetagname>[a-zA-Z0-9]+)\s*\]`)

View File

@ -505,10 +505,6 @@ header {
.user-options { .user-options {
position: relative; position: relative;
a {
@extend .pa2;
}
} }
.login, .register { .login, .register {
@ -571,6 +567,8 @@ footer {
-moz-text-size-adjust:auto; -moz-text-size-adjust:auto;
-webkit-text-size-adjust:auto; -webkit-text-size-adjust:auto;
text-size-adjust:auto; text-size-adjust:auto;
margin: 0.6rem 0;
} }
.description { .description {

View File

@ -1,3 +1,7 @@
form {
margin: 0;
}
.radio, .checkbox { .radio, .checkbox {
$size: 1.3rem; $size: 1.3rem;
$margin: 0.5rem; $margin: 0.5rem;

View File

@ -1,7 +1,7 @@
.notice { .notice {
@include usevar(color, notice-text-color); @include usevar(color, notice-text-color);
@extend .ph3, .pv2; @extend .ph3, .pv2;
@extend .br2; @extend .br2-ns;
a { a {
@include usevar(color, notice-text-color); @include usevar(color, notice-text-color);

View File

@ -273,7 +273,7 @@ func TimelineItemsToJSON(items []TimelineItem) string {
builder.WriteString(`",`) builder.WriteString(`",`)
builder.WriteString(`"owner_name":"`) 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(`",`)
builder.WriteString(`"owner_avatar":"`) builder.WriteString(`"owner_avatar":"`)

View File

@ -93,6 +93,9 @@
{{ end }} {{ end }}
<a class="reply action button" href="{{ .ReplyUrl }}" title="Reply">&hookrightarrow;</a>&nbsp; <a class="reply action button" href="{{ .ReplyUrl }}" title="Reply">&hookrightarrow;</a>&nbsp;
{{ end }} {{ end }}
<span class="postid">
<a name="{{ .ID }}" href="{{ .Url }}">#{{ .ID }}</a>
</span>
</div> </div>
{{ end }} {{ end }}
</div> </div>

View File

@ -3,7 +3,6 @@
{{ define "extrahead" }} {{ define "extrahead" }}
{{/* TODO: These are no longer useful? */}} {{/* TODO: These are no longer useful? */}}
<link rel="stylesheet" href="{{ static "editor.css" }}" /> <link rel="stylesheet" href="{{ static "editor.css" }}" />
<script src="{{ static "util.js" }}"></script>
<script src="{{ static "editor.js" }}"></script> <script src="{{ static "editor.js" }}"></script>
<script src="{{ static "go_wasm_exec.js" }}"></script> <script src="{{ static "go_wasm_exec.js" }}"></script>

View File

@ -34,13 +34,12 @@
<div class="dn db-l w-60 pv2"> <div class="dn db-l w-60 pv2">
<div class="aspect-ratio--1x1 contain bg-center" style="background-image:url('{{ .Author.AvatarUrl }}');"></div> <div class="aspect-ratio--1x1 contain bg-center" style="background-image:url('{{ .Author.AvatarUrl }}');"></div>
</div> </div>
{{/* TODO: Aggregate user data
<div class="c--dim f7"> <div class="c--dim f7">
{{ post.author.posts }} posts {{ .AuthorNumPosts }} posts
{% if post.author.public_projects.values|length > 0 %} {{ if gt .AuthorNumProjects 0 }}
/ {{ post.author.public_projects.values|length }} project{%if post.author.public_projects.values|length > 1 %}s{% endif %} / {{ .AuthorNumProjects }} project{{ if gt .AuthorNumProjects 1 }}s{{ end }}
{% endif %} {{ end }}
</div> */}} </div>
<!-- Large badges --> <!-- Large badges -->
<div class="dn db-l pv2"> <div class="dn db-l pv2">
{{ if .Author.IsStaff }} {{ if .Author.IsStaff }}

View File

@ -6,7 +6,7 @@
</div> </div>
<div class="pl3 flex flex-column"> <div class="pl3 flex flex-column">
<div> <div>
<a class="username" href="{{ .Author.ProfileUrl }}" target="_blank">{{ .Author.Username }}</a> {{/* TODO: Text scale stuff? Seems unnecessary. */}} <a class="username" href="{{ .Author.ProfileUrl }}" target="_blank">{{ .Author.Username }}</a>
<!-- Mobile badges --> <!-- Mobile badges -->
<div class="di ph1"> <div class="di ph1">
{{ if .Author.IsStaff }} {{ if .Author.IsStaff }}

View File

@ -2,13 +2,16 @@
<div class="user-options flex justify-center justify-end-ns"> <div class="user-options flex justify-center justify-end-ns">
{{ if .User }} {{ if .User }}
{{ if .User.IsStaff }} {{ if .User.IsStaff }}
<a class="admin-panel" href="{{ .Header.AdminUrl }}"><span class="icon-settings"> Admin</span></a> <a class="admin-panel pa2" href="{{ .Header.AdminUrl }}"><span class="icon-settings"> Admin</span></a>
{{ end }} {{ end }}
<a class="username settings" href="{{ .Header.UserSettingsUrl }}"><span class="icon-settings"></span> {{ .User.Username }}</a> <div>
<a class="logout" href="{{ .Header.LogoutActionUrl }}"><span class="icon-logout"></span> Log Out</a> <a class="dib pv2 pl2" href="{{ .Header.UserProfileUrl }}">{{ .User.Username }}</a>
<a class="dib pv2 pr2" href="{{ .Header.UserSettingsUrl }}">(settings)</a>
</div>
<a class="logout pa2" href="{{ .Header.LogoutActionUrl }}"><span class="icon-logout"></span> Log Out</a>
{{ else }} {{ else }}
<a class="register" id="register-link" href="{{ .Header.RegisterUrl }}">Register</a> <a class="register pa2" id="register-link" href="{{ .Header.RegisterUrl }}">Register</a>
<a class="login" id="login-link" href="{{ .LoginPageUrl }}">Log in</a> <a class="login pa2" id="login-link" href="{{ .LoginPageUrl }}">Log in</a>
<div id="login-popup"> <div id="login-popup">
<form action="{{ .Header.LoginActionUrl }}" method="post" class="ma0"> <form action="{{ .Header.LoginActionUrl }}" method="post" class="ma0">
<input type="text" name="username" class="w-100" value="" placeholder="Username" /> <input type="text" name="username" class="w-100" value="" placeholder="Username" />
@ -58,7 +61,7 @@
</div> </div>
<form onsubmit="this.querySelector('input[name=q]').value = this.querySelector('#searchstring').value + ' site:handmade.network';" class="dn ma0 flex-l flex-column justify-center items-end" method="GET" action="{{ .Header.SearchActionUrl }}" target="_blank"> <form onsubmit="this.querySelector('input[name=q]').value = this.querySelector('#searchstring').value + ' site:handmade.network';" class="dn ma0 flex-l flex-column justify-center items-end" method="GET" action="{{ .Header.SearchActionUrl }}" target="_blank">
<input type="hidden" name="q" /> <input type="hidden" name="q" />
<input class="site-search bn lite pa2 fira" type="text" id="searchstring" value="" placeholder="Search with DuckDuckGo" size="17" /> <input class="site-search bn lite pa2 fira" type="text" id="searchstring" value="" placeholder="Search with DuckDuckGo" size="18" />
<input id="search_button_homepage" type="submit" value="Go"/> <input id="search_button_homepage" type="submit" value="Go"/>
</form> </form>
</div> </div>

View File

@ -12,7 +12,7 @@
{{ end }} {{ end }}
{{ end }} {{ end }}
{{ if .Title }} {{ if .Title }}
<title>{{ .Title }} | Handmade Network</title> {{/* TODO: Some parts of the site replace "Handmade Network" with other things like "4coder Forums". */}} <title>{{ .Title }} | Handmade Network</title>
{{ else }} {{ else }}
<title>Handmade Network</title> <title>Handmade Network</title>
{{ end }} {{ end }}

View File

@ -47,7 +47,6 @@
</div> </div>
</div> </div>
{{ end }} {{ end }}
{{/* TODO(asaf): Add timeline items for project */}}
</div> </div>
<div class="sidebar flex-shrink-0 mw6 w-30-l self-center self-start-l mh3 mh0-ns ml3-l overflow-hidden"> <div class="sidebar flex-shrink-0 mw6 w-30-l self-center self-start-l mh3 mh0-ns ml3-l overflow-hidden">
<div class="content-block"> <div class="content-block">

View File

@ -1,7 +1,7 @@
{{ template "base.html" . }} {{ template "base.html" . }}
{{ define "content" }} {{ define "content" }}
<div class="description"> <div class="description ph2 ph0-ns">
<p> <p>
<span class="big">Hi there, {{ if .User }}{{ .User.Name }}{{ else }}visitor{{ end }}!</span> <span class="big">Hi there, {{ if .User }}{{ .User.Name }}{{ else }}visitor{{ end }}!</span>
</p> </p>

View File

@ -39,6 +39,7 @@ func (bd *BaseData) AddImmediateNotice(class, content string) {
type Header struct { type Header struct {
AdminUrl string AdminUrl string
UserProfileUrl string
UserSettingsUrl string UserSettingsUrl string
LoginActionUrl string LoginActionUrl string
LogoutActionUrl string LogoutActionUrl string
@ -90,6 +91,9 @@ type Post struct {
Content template.HTML Content template.HTML
PostDate time.Time PostDate time.Time
AuthorNumPosts int
AuthorNumProjects int
Editor *User Editor *User
EditDate time.Time EditDate time.Time
EditReason string EditReason string

View File

@ -204,7 +204,7 @@ func BlogPostRedirectToThread(c *RequestContext) ResponseData {
thread := FetchThread(c.Context(), c.Conn, cd.ThreadID) 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) return c.Redirect(threadUrl, http.StatusFound)
} }

View File

@ -5,7 +5,6 @@ import (
"net/http" "net/http"
"time" "time"
"git.handmade.network/hmn/hmn/src/auth"
"git.handmade.network/hmn/hmn/src/config" "git.handmade.network/hmn/hmn/src/config"
"git.handmade.network/hmn/hmn/src/db" "git.handmade.network/hmn/hmn/src/db"
"git.handmade.network/hmn/hmn/src/discord" "git.handmade.network/hmn/hmn/src/discord"
@ -22,25 +21,16 @@ func DiscordOAuthCallback(c *RequestContext) ResponseData {
if state != c.CurrentSession.CSRFToken { if state != c.CurrentSession.CSRFToken {
// CSRF'd!!!! // 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?") 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 := c.Redirect("/", http.StatusSeeOther)
res.SetCookie(auth.DeleteSessionCookie) logoutUser(c, &res)
return 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 != "" { if errCode := query.Get("error"); errCode != "" {
// TODO: actually handle these errors
if errCode == "access_denied" { if errCode == "access_denied" {
// This occurs when the user cancels. Just go back to the profile page. // This occurs when the user cancels. Just go back to the profile page.
return c.Redirect(hmnurl.BuildUserSettings("discord"), http.StatusSeeOther) 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") code := query.Get("code")
res, err := discord.ExchangeOAuthCode(c.Context(), code, hmnurl.BuildDiscordOAuthCallback()) res, err := discord.ExchangeOAuthCode(c.Context(), code, hmnurl.BuildDiscordOAuthCallback())
if err != nil { if err != nil {

View File

@ -98,7 +98,7 @@ func Feed(c *RequestContext) ResponseData {
BaseData: baseData, BaseData: baseData,
AtomFeedUrl: hmnurl.BuildAtomFeed(), AtomFeedUrl: hmnurl.BuildAtomFeed(),
MarkAllReadUrl: hmnurl.BuildForumMarkRead(0), MarkAllReadUrl: hmnurl.BuildForumMarkRead(c.CurrentProject.Slug, 0),
Posts: posts, Posts: posts,
Pagination: pagination, Pagination: pagination,
}, c.Perf) }, c.Perf)

View File

@ -1,6 +1,7 @@
package website package website
import ( import (
"context"
"fmt" "fmt"
"net/http" "net/http"
"strconv" "strconv"
@ -9,6 +10,7 @@ import (
"git.handmade.network/hmn/hmn/src/db" "git.handmade.network/hmn/hmn/src/db"
"git.handmade.network/hmn/hmn/src/hmnurl" "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/models"
"git.handmade.network/hmn/hmn/src/oops" "git.handmade.network/hmn/hmn/src/oops"
"git.handmade.network/hmn/hmn/src/templates" "git.handmade.network/hmn/hmn/src/templates"
@ -283,7 +285,7 @@ func Forum(c *RequestContext) ResponseData {
res.MustWriteTemplate("forum.html", forumData{ res.MustWriteTemplate("forum.html", forumData{
BaseData: baseData, BaseData: baseData,
NewThreadUrl: hmnurl.BuildForumNewThread(c.CurrentProject.Slug, currentSubforumSlugs, false), NewThreadUrl: hmnurl.BuildForumNewThread(c.CurrentProject.Slug, currentSubforumSlugs, false),
MarkReadUrl: hmnurl.BuildForumMarkRead(cd.SubforumID), MarkReadUrl: hmnurl.BuildForumMarkRead(c.CurrentProject.Slug, cd.SubforumID),
Threads: threads, Threads: threads,
Pagination: templates.Pagination{ Pagination: templates.Pagination{
Current: page, Current: page,
@ -470,6 +472,7 @@ func ForumThread(c *RequestContext) ResponseData {
) )
c.Perf.EndBlock() c.Perf.EndBlock()
c.Perf.StartBlock("TEMPLATE", "Create template posts")
var posts []templates.Post var posts []templates.Post
for _, p := range postsAndStuff { for _, p := range postsAndStuff {
post := templates.PostToTemplate(&p.Post, p.Author, c.Theme) post := templates.PostToTemplate(&p.Post, p.Author, c.Theme)
@ -482,8 +485,11 @@ func ForumThread(c *RequestContext) ResponseData {
post.ReplyPost = &reply post.ReplyPost = &reply
} }
addAuthorCountsToPost(c.Context(), c.Conn, &post)
posts = append(posts, post) posts = append(posts, post)
} }
c.Perf.EndBlock()
// Update thread last read info // Update thread last read info
if c.CurrentUser != nil { if c.CurrentUser != nil {
@ -774,9 +780,11 @@ func ForumPostEditSubmit(c *RequestContext) ResponseData {
defer tx.Rollback(c.Context()) defer tx.Rollback(c.Context())
c.Req.ParseForm() c.Req.ParseForm()
// TODO(ben): Validation
unparsed := c.Req.Form.Get("body") unparsed := c.Req.Form.Get("body")
editReason := c.Req.Form.Get("editreason") 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) 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.EditUrl = hmnurl.BuildForumPostEdit(projectSlug, subforums, threadId, postId)
p.ReplyUrl = hmnurl.BuildForumPostReply(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
}
}

View File

@ -14,38 +14,49 @@ import (
"git.handmade.network/hmn/hmn/src/oops" "git.handmade.network/hmn/hmn/src/oops"
) )
// If a helper method returns this, you should call RejectRequest with the value. type SaveImageFileResult struct {
type RejectRequestError error ImageFileID int
ValidationError string
FatalError error
}
/* /*
Reads an image file from form data and saves it to the filesystem and the database. 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. 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) img, header, err := c.Req.FormFile(fileFieldName)
filename := "" filename := ""
width := 0 width := 0
height := 0 height := 0
if err != nil && !errors.Is(err, http.ErrMissingFile) { 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 != nil {
if header.Size > maxSize { 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 { } else {
c.Perf.StartBlock("IMAGE", "Decoding image") c.Perf.StartBlock("IMAGE", "Decoding image")
config, format, err := image.DecodeConfig(img) config, format, err := image.DecodeConfig(img)
c.Perf.EndBlock() c.Perf.EndBlock()
if err != nil { if err != nil {
return 0, RejectRequestError(errors.New("Image type not supported")) return SaveImageFileResult{
ValidationError: "Image type not supported",
}
} }
width = config.Width width = config.Width
height = config.Height height = config.Height
if width == 0 || height == 0 { 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) 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") c.Perf.StartBlock("IMAGE", "Writing image file")
file, err := os.Create(storageFilename) file, err := os.Create(storageFilename)
if err != nil { 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) img.Seek(0, io.SeekStart)
_, err = io.Copy(file, img) _, err = io.Copy(file, img)
if err != nil { 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() file.Close()
img.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, filename, header.Size, hex.EncodeToString(sha1sum), false, width, height,
).Scan(&imageId) ).Scan(&imageId)
if err != nil { 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{}
} }

View File

@ -334,7 +334,7 @@ func Index(c *RequestContext) ResponseData {
Url: hmnurl.BuildBlogThread(models.HMNProjectSlug, newsPostResult.Thread.ID, newsPostResult.Thread.Title), Url: hmnurl.BuildBlogThread(models.HMNProjectSlug, newsPostResult.Thread.ID, newsPostResult.Thread.Title),
User: templates.UserToTemplate(&newsPostResult.User, c.Theme), User: templates.UserToTemplate(&newsPostResult.User, c.Theme),
Date: newsPostResult.Post.PostDate, Date: newsPostResult.Post.PostDate,
Unread: true, // TODO Unread: true,
Content: template.HTML(newsPostResult.PostVersion.TextParsed), Content: template.HTML(newsPostResult.PostVersion.TextParsed),
}, },
PostColumns: cols, PostColumns: cols,

View File

@ -147,17 +147,14 @@ func PodcastEditSubmit(c *RequestContext) ResponseData {
} }
defer tx.Rollback(c.Context()) 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())) imageSaveResult := SaveImageFile(c, tx, "podcast_image", maxFileSize, fmt.Sprintf("podcast/%s/logo%d", c.CurrentProject.Slug, time.Now().UTC().Unix()))
if err != nil { if imageSaveResult.ValidationError != "" {
var rejectErr RejectRequestError return RejectRequest(c, imageSaveResult.ValidationError)
if errors.As(err, &rejectErr) { } else if imageSaveResult.FatalError != nil {
return RejectRequest(c, rejectErr.Error()) return c.ErrorResponse(http.StatusInternalServerError, oops.New(imageSaveResult.FatalError, "Failed to save podcast image"))
} else {
return c.ErrorResponse(http.StatusInternalServerError, oops.New(err, "Failed to save podcast image"))
}
} }
if imageId != 0 { if imageSaveResult.ImageFileID != 0 {
_, err = tx.Exec(c.Context(), _, err = tx.Exec(c.Context(),
` `
UPDATE handmade_podcast UPDATE handmade_podcast
@ -169,7 +166,7 @@ func PodcastEditSubmit(c *RequestContext) ResponseData {
`, `,
title, title,
description, description,
imageId, imageSaveResult.ImageFileID,
podcastResult.Podcast.ID, podcastResult.Podcast.ID,
) )
if err != nil { if err != nil {

View File

@ -107,8 +107,6 @@ func (r *Router) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
return 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)) panic(fmt.Sprintf("Path '%s' did not match any routes! Make sure to register a wildcard route to act as a 404.", path))
} }

View File

@ -112,13 +112,8 @@ func NewWebsiteRoutes(longRequestContext context.Context, conn *pgxpool.Pool, pe
if csrfToken != c.CurrentSession.CSRFToken { if csrfToken != c.CurrentSession.CSRFToken {
c.Logger.Warn().Str("userId", c.CurrentUser.Username).Msg("user failed CSRF validation - potential attack?") 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 := c.Redirect("/", http.StatusSeeOther)
res.SetCookie(auth.DeleteSessionCookie) logoutUser(c, &res)
return res return res
} }
@ -275,7 +270,7 @@ func getBaseData(c *RequestContext) templates.BaseData {
episodeGuideUrl = hmnurl.BuildEpisodeList(c.CurrentProject.Slug, defaultTopic) episodeGuideUrl = hmnurl.BuildEpisodeList(c.CurrentProject.Slug, defaultTopic)
} }
return templates.BaseData{ baseData := templates.BaseData{
Theme: c.Theme, Theme: c.Theme,
CurrentUrl: c.FullUrl(), CurrentUrl: c.FullUrl(),
@ -322,6 +317,12 @@ func getBaseData(c *RequestContext) templates.BaseData {
SitemapUrl: hmnurl.BuildSiteMap(), SitemapUrl: hmnurl.BuildSiteMap(),
}, },
} }
if c.CurrentUser != nil {
baseData.Header.UserProfileUrl = hmnurl.BuildUserProfile(c.CurrentUser.Username)
}
return baseData
} }
func buildDefaultOpenGraphItems(project *models.Project) []templates.OpenGraphItem { func buildDefaultOpenGraphItems(project *models.Project) []templates.OpenGraphItem {

View File

@ -454,27 +454,24 @@ func UserSettingsSave(c *RequestContext) ResponseData {
} }
// Update avatar // Update avatar
_, err = SaveImageFile(c, tx, "avatar", 1*1024*1024, fmt.Sprintf("members/avatars/%s", c.CurrentUser.Username)) imageSaveResult := SaveImageFile(c, tx, "avatar", 1*1024*1024, fmt.Sprintf("members/avatars/%s", c.CurrentUser.Username))
if err != nil { if imageSaveResult.ValidationError != "" {
var rejectErr RejectRequestError return RejectRequest(c, imageSaveResult.ValidationError)
if errors.As(err, &rejectErr) { } else if imageSaveResult.FatalError != nil {
return RejectRequest(c, rejectErr.Error()) return c.ErrorResponse(http.StatusInternalServerError, oops.New(imageSaveResult.FatalError, "failed to save new avatar"))
} else {
return c.ErrorResponse(http.StatusInternalServerError, oops.New(err, "failed to save new avatar"))
}
} }
// TODO: Success message
err = tx.Commit(c.Context()) err = tx.Commit(c.Context())
if err != nil { if err != nil {
return c.ErrorResponse(http.StatusInternalServerError, oops.New(err, "failed to save user settings")) 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 { func updatePassword(c *RequestContext, tx pgx.Tx, old, new, confirm string) *ResponseData {
if new != confirm { if new != confirm {
res := RejectRequest(c, "Your password and password confirmation did not match.") res := RejectRequest(c, "Your password and password confirmation did not match.")