From 75f88e3b1aebfe9b7b2c25b6de32c846a9fd53c9 Mon Sep 17 00:00:00 2001 From: Max Freedom Pollard <272618364+MaxFreedomPollard@users.noreply.github.com> Date: Mon, 7 Sep 2026 06:28:01 -0400 Subject: [PATCH] fix: comment url swaps title and answer id CommentURL called AnswerURL(permalink, siteUrl, questionID, answerID, title), but AnswerURL takes (permalink, siteUrl, questionID, title, answerID), so the last two arguments were reversed for every comment left on an answer. The answer route is /questions/:id/:title/:answerid, so the answer id landed in the title slot and the title landed in the answer id slot. Under the numeric permalink settings the title was run through uid.DeShortID and came out as an unrelated number. Under the short id settings uid.EnShortID returned it unchanged, so the raw title, spaces and all, was written into the path. The broken link reaches the new comment notification email built in internal/service/export/email_service.go and the CommentUrl handed to notification plugins in internal/service/notification_common/notification.go. A comment on a question takes the other branch and was already correct. Pass title and answerID in the order AnswerURL declares them, and add pkg/display/url_test.go covering both branches across all four permalink settings. --- pkg/display/url.go | 2 +- pkg/display/url_test.go | 115 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 pkg/display/url_test.go diff --git a/pkg/display/url.go b/pkg/display/url.go index 11574f0a5..fc7fbdbec 100644 --- a/pkg/display/url.go +++ b/pkg/display/url.go @@ -54,7 +54,7 @@ func AnswerURL(permalink int, siteUrl, questionID, title, answerID string) strin // CommentURL get comment url func CommentURL(permalink int, siteUrl, questionID, title, answerID, commentID string) string { if len(answerID) > 0 { - return AnswerURL(permalink, siteUrl, questionID, answerID, title) + "?commentId=" + commentID + return AnswerURL(permalink, siteUrl, questionID, title, answerID) + "?commentId=" + commentID } return QuestionURL(permalink, siteUrl, questionID, title) + "?commentId=" + commentID } diff --git a/pkg/display/url_test.go b/pkg/display/url_test.go new file mode 100644 index 000000000..dffbb3957 --- /dev/null +++ b/pkg/display/url_test.go @@ -0,0 +1,115 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package display + +import ( + "testing" + + "github.com/apache/answer/internal/base/constant" + "github.com/stretchr/testify/assert" +) + +const ( + testSiteURL = "https://example.com" + testQuestionID = "10010000000000001" + testTitle = "How to install Answer" + testAnswerID = "10020000000000002" + testCommentID = "10030000000000003" +) + +// CommentURL passed title and answerID to AnswerURL in the wrong order, so a +// comment on an answer linked to a URL built from the title where the answer id +// belongs. Under the short id permalinks the raw title, spaces and all, ended up +// in the path. +func TestCommentURLOnAnswer(t *testing.T) { + cases := []struct { + name string + permalink int + want string + }{ + { + name: "question id and title", + permalink: constant.PermalinkQuestionIDAndTitle, + want: "https://example.com/questions/10010000000000001/how-to-install-answer/10020000000000002?commentId=10030000000000003", + }, + { + name: "question id", + permalink: constant.PermalinkQuestionID, + want: "https://example.com/questions/10010000000000001/10020000000000002?commentId=10030000000000003", + }, + { + name: "question id and title by short id", + permalink: constant.PermalinkQuestionIDAndTitleByShortID, + want: "https://example.com/questions/D1D1/how-to-install-answer/E1E1?commentId=10030000000000003", + }, + { + name: "question id by short id", + permalink: constant.PermalinkQuestionIDByShortID, + want: "https://example.com/questions/D1D1/E1E1?commentId=10030000000000003", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := CommentURL(tc.permalink, testSiteURL, testQuestionID, testTitle, testAnswerID, testCommentID) + assert.Equal(t, tc.want, got) + assert.Equal(t, + AnswerURL(tc.permalink, testSiteURL, testQuestionID, testTitle, testAnswerID)+"?commentId="+testCommentID, + got, + "a comment on an answer should be the answer URL plus the comment query") + }) + } +} + +// A comment on the question itself takes the other branch, which was already +// correct. Pin it so the fix above stays scoped to the answer branch. +func TestCommentURLOnQuestion(t *testing.T) { + cases := []struct { + name string + permalink int + want string + }{ + { + name: "question id and title", + permalink: constant.PermalinkQuestionIDAndTitle, + want: "https://example.com/questions/10010000000000001/how-to-install-answer?commentId=10030000000000003", + }, + { + name: "question id", + permalink: constant.PermalinkQuestionID, + want: "https://example.com/questions/10010000000000001?commentId=10030000000000003", + }, + { + name: "question id and title by short id", + permalink: constant.PermalinkQuestionIDAndTitleByShortID, + want: "https://example.com/questions/D1D1/how-to-install-answer?commentId=10030000000000003", + }, + { + name: "question id by short id", + permalink: constant.PermalinkQuestionIDByShortID, + want: "https://example.com/questions/D1D1?commentId=10030000000000003", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := CommentURL(tc.permalink, testSiteURL, testQuestionID, testTitle, "", testCommentID) + assert.Equal(t, tc.want, got) + }) + } +}