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

Use short posts and comments IDs in the URLs #1639

Merged
merged 9 commits into from
Aug 24, 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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [1.123.0] - Not released
### Added
- Links to posts and comments are now shorter: e.g. `/user/4a39b8` (a post) or
`/groupname/f482e5#ad2b` (a comment).
- All public URLs and links to posts and comments are now in short format.
- Addresses of posts and comments with leading slash (`/user/...`) are now
parses in texts as active links.
- Old-fashion links, with long UIDs, are still fully supported.

### Changed
- Spoiler tag can now contain line feeds. In addition, user text formatting
utilizes simpler HTML, with fewer wrappers. Visually, the output for the user
Expand Down
2 changes: 1 addition & 1 deletion index.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<meta name="viewport" content="width=device-width,initial-scale=1,maximum-scale=1" />

<!-- Support for FB OpenGraph and Twitter -->
<!--#if expr="${request_uri} = /^\/([a-zA-Z0-9]+)\/([a-fA-F0-9\-]{36})/" -->
<!--#if expr="${request_uri} = /^\/([a-zA-Z0-9]+)\/([a-fA-F0-9\-]{36}|[a-fA-F0-9]{4,6})/" -->
<!--#set var="post_id" value="$2" -->
<!--#include virtual="/v2/posts-opengraph/${post_id}" -->
<!--#endif -->
Expand Down
3 changes: 2 additions & 1 deletion src/app.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,8 @@ function isPostPath({ params: { postId, userName } }) {
// old groups can have up to 27 characters in username
return (
/^[a-z\d-]{3,30}$/i.test(userName) &&
/^[a-f\d]{8}-[a-f\d]{4}-4[a-f\d]{3}-[89ab][a-f\d]{3}-[a-f\d]{12}$/i.test(postId)
(/^[a-f\d]{8}-[a-f\d]{4}-4[a-f\d]{3}-[89ab][a-f\d]{3}-[a-f\d]{12}$/i.test(postId) ||
/^[a-f\d]{6,10}$/i.test(postId)) // Short post ID
);
}

Expand Down
21 changes: 16 additions & 5 deletions src/components/notifications.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@ const getAuthorName = ({ postAuthor, createdUser, group }) => {
return createdUser.username;
};

const generatePostUrl = ({ post_id, ...event }) => `/${getAuthorName(event)}/${post_id}`;
const generateCommentUrl = ({ post_id, comment_id, ...event }) =>
`/${getAuthorName(event)}/${post_id}#comment-${comment_id}`;
const generatePostUrl = ({ post_id, shortPostId, ...event }) =>
`/${getAuthorName(event)}/${shortPostId ?? post_id}`;
const generateCommentUrl = ({ post_id, comment_id, shortPostId, shortCommentId, ...event }) =>
`/${getAuthorName(event)}/${shortPostId ?? post_id}#${
shortCommentId ? shortCommentId : `comment-${comment_id}`
}`;
const postLink = (event, fallback = 'deleted post') =>
event.post_id ? <Link to={generatePostUrl(event)}>post</Link> : fallback;
const directPostLink = (event) =>
Expand Down Expand Up @@ -421,14 +424,22 @@ function Notification({ event }) {
: contentSource === 'comment'
? allComments[event.comment_id]?.body
: null;
const mainLink = mainEventLink(event);
const eventWithShortIds = useMemo(
() => ({
...event,
shortPostId: allPosts[event.post_id]?.shortId,
shortCommentId: allComments[event.comment_id]?.shortId,
}),
[allComments, allPosts, event],
);
const mainLink = mainEventLink(eventWithShortIds);
return (
<div className={`single-notification ${notificationClasses[event.event_type] || ''}`}>
<div className="single-notification__picture">
<UserPicture user={userPictureSource(event)} size={40} loading="lazy" />
</div>
<div className="single-notification__headline">
{(notificationTemplates[event.event_type] || nop)(event)}
{(notificationTemplates[event.event_type] || nop)(eventWithShortIds)}
</div>
<div className="single-notification__content">
{content ? (
Expand Down
4 changes: 2 additions & 2 deletions src/components/post/post-comment-preview.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export function PostCommentPreview({
{comment && frontPreferences.comments.showTimestamps && (
<span className="comment-tail__item">
<Link
to={`${postUrl}#comment-${comment.id}`}
to={`${postUrl}#${comment.shortId}`}
className="comment-tail__timestamp"
onClick={onClick}
>
Expand Down Expand Up @@ -172,7 +172,7 @@ export function PostCommentPreview({
</Expandable>
)}
<div className={styles['actions']}>
<Link to={`${postUrl}#comment-${comment?.id}`} onClick={onClick}>
<Link to={`${postUrl}#${comment?.shortId}`} onClick={onClick}>
Go to comment
</Link>
</div>
Expand Down
4 changes: 2 additions & 2 deletions src/components/post/post-comment.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ class PostComment extends Component {
getBackwardIdx={this.backwardIdx}
createdAt={this.props.createdAt}
updatedAt={this.props.updatedAt}
permalink={`${this.props.entryUrl}#comment-${this.props.id}`}
permalink={`${this.props.entryUrl}#${this.props.shortId}`}
likesCount={this.props.likes}
setMenuOpener={this.setMoreMenuOpener}
onMenuOpened={this.onMoreMenuOpened}
Expand All @@ -240,7 +240,7 @@ class PostComment extends Component {
{(this.props.showTimestamps || this.props.forceAbsTimestamps) && (
<span className="comment-tail__item">
<Link
to={`${this.props.entryUrl}#comment-${this.props.id}`}
to={`${this.props.entryUrl}#${this.props.shortId}`}
className="comment-tail__timestamp"
>
<TimeDisplay
Expand Down
4 changes: 2 additions & 2 deletions src/components/select-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const getCommentId = (hash) => {
if (!hash) {
return '';
}
return hash.replace('#comment-', '');
return hash.startsWith('#comment-') ? hash.replace('#comment-', '') : hash.replace('#', '');
};

export const joinPostData = (state) => (postId) => {
Expand Down Expand Up @@ -161,7 +161,7 @@ export const joinPostData = (state) => (postId) => {
isEditable: user.id === comment.createdBy,
isDeletable: isModeratable || isModeratable,
likesList: selectCommentLikes(state, commentId),
highlightedFromUrl: commentId === hashedCommentId,
highlightedFromUrl: commentId === hashedCommentId || comment.shortId === hashedCommentId,
};
})
.filter(Boolean);
Expand Down
8 changes: 4 additions & 4 deletions src/components/single-post.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import { PostContextProvider } from './post/post-context';

class SinglePostHandler extends Component {
UNSAFE_componentWillReceiveProps(nextProps) {
const { post, router } = nextProps;
if (!post) {
const { post, router, routeLoadingState } = nextProps;
if (!post || routeLoadingState) {
return;
}
const { pathname, search, hash } = router.location;
Expand Down Expand Up @@ -84,13 +84,13 @@ class SinglePostHandler extends Component {
}

function selectState(state) {
const { boxHeader, user } = state;
const { boxHeader, user, routeLoadingState } = state;

const post = joinPostData(state)(state.singlePostId);
const viewState = state.postsViewState[state.singlePostId];
const errorString = viewState ? viewState.errorString || 'Unknown error' : null;

return { post, user, boxHeader, errorString };
return { post, user, boxHeader, errorString, routeLoadingState };
}

function selectActions(dispatch) {
Expand Down
6 changes: 3 additions & 3 deletions src/redux/reducers.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ export function feedViewState(state = initFeed, action) {
};
}
case response(ActionTypes.GET_SINGLE_POST): {
const { postId } = action.request;
const { id: postId } = action.payload.posts;
return {
...initFeed,
entries: [postId],
Expand Down Expand Up @@ -1318,8 +1318,8 @@ export function singlePostId(state = null, action) {
if (ActionHelpers.isFeedRequest(action)) {
return null;
}
if (action.type == request(ActionTypes.GET_SINGLE_POST)) {
return action.payload.postId;
if (action.type == response(ActionTypes.GET_SINGLE_POST)) {
return action.payload.posts.id;
}
return state;
}
Expand Down
2 changes: 1 addition & 1 deletion src/utils/canonical-uri.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ export function canonicalURI(post) {
if (post.recipients.length > 0 && !post.recipients.some((r) => r.type === 'user')) {
urlName = post.recipients[0].username;
}
return `/${encodeURIComponent(urlName)}/${encodeURIComponent(post.id)}`;
return `/${encodeURIComponent(urlName)}/${encodeURIComponent(post.shortId)}`;
}
42 changes: 37 additions & 5 deletions src/utils/parse-text.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,29 +28,43 @@ const {
siteDomains,
} = CONFIG;

const shortLinkRe = /\/[a-z\d-]{3,35}\/[\da-f]{6,10}(?:#[\da-f]{4,6})?/gi;
const shortLinkExactRe = /^\/[a-z\d-]{3,35}\/[\da-f]{6,10}(?:#[\da-f]{4,6})?$/i;

export class Link extends TLink {
localDomains = [];
hostname = null;
path = '/';
link;
isShort = false;

constructor(link, localDomains) {
super(link.offset, link.text);
this.link = link;
this.localDomains = localDomains;
this.isShort = shortLinkExactRe.test(link.text);

const m = this.link.href.match(/^https?:\/\/([^/]+)(.*)/i);
const m = this.href.match(/^https?:\/\/([^/]+)(.*)/i);
if (m) {
this.hostname = m[1].toLowerCase();
this.path = m[2] || '/';
} else if (this.isShort) {
this.path = link.text;
}
}

get href() {
return this.link.href;
get pretty() {
if (this.isShort || (this.isLocal && shortLinkExactRe.test(this.path))) {
return this.path;
}
return super.pretty;
}

get isLocal() {
// Short links are always local
if (this.isShort) {
return true;
}
const p = this.localDomains.indexOf(this.hostname);
if (p === -1) {
return false;
Expand All @@ -61,7 +75,7 @@ export class Link extends TLink {
}

// Other domains in localDomains list are alternative frontends or mirrors.
// Such links should be treated as remote if theay lead to the domain root.
// Such links should be treated as remote if they lead to the domain root.
return this.path !== '/';
}

Expand Down Expand Up @@ -111,7 +125,7 @@ export class RedditLink extends TLink {
}

const redditLinks = () => {
const beforeChars = new RegExp(`[${wordAdjacentChars}]`);
const beforeChars = new RegExp(`[${wordAdjacentChars.clone().removeChars('/')}]`);
const afterChars = new RegExp(`[${wordAdjacentChars.clone().removeChars('/')}]`);
return byRegexp(/\/?r\/[A-Za-z\d]\w{1,20}/g, (offset, text, match) => {
const charBefore = match.input.charAt(offset - 1);
Expand All @@ -127,6 +141,23 @@ const redditLinks = () => {
});
};

const shortLinks = () => {
const beforeChars = new RegExp(`[${wordAdjacentChars.clone().removeChars('/')}]`);
const afterChars = new RegExp(`[${wordAdjacentChars.clone().removeChars('/')}]`);
return byRegexp(shortLinkRe, (offset, text, match) => {
const charBefore = match.input.charAt(offset - 1);
const charAfter = match.input.charAt(offset + text.length);
if (charBefore !== '' && !beforeChars.test(charBefore)) {
return null;
}
if (charAfter !== '' && !afterChars.test(charAfter)) {
return null;
}

return new TLink(offset, text);
});
};

export class LineBreak extends Token {}
export class ParagraphBreak extends Token {}

Expand All @@ -151,6 +182,7 @@ const tokenize = withText(
mentions(),
foreignMentions(),
links({ tldRe }),
shortLinks(),
redditLinks(),
arrows(/\u2191+|\^([1-9]\d*|\^*)/g),
tokenizerStartSpoiler,
Expand Down