UX: redesign admin permalinks page (#29634)

Redesign the permalinks page to follow the UX guide. In addition, the ability to edit permalinks was added.

This change includes:
- move to RestModel
- added Validations
- update endpoint and clear old values after the update
- system specs and improvements for unit tests
This commit is contained in:
Krzysztof Kotlarek 2024-11-14 10:03:58 +11:00 committed by GitHub
parent b37f6f1edb
commit 42b1ca8f78
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
29 changed files with 924 additions and 239 deletions

View File

@ -0,0 +1,7 @@
import RestAdapter from "discourse/adapters/rest";
export default class Permalink extends RestAdapter {
basePath() {
return "/admin/";
}
}

View File

@ -0,0 +1,245 @@
import Component from "@glimmer/component";
import { cached } from "@glimmer/tracking";
import { inject as controller } from "@ember/controller";
import { action } from "@ember/object";
import { service } from "@ember/service";
import { isEmpty } from "@ember/utils";
import { eq } from "truth-helpers";
import BackButton from "discourse/components/back-button";
import Form from "discourse/components/form";
import { popupAjaxError } from "discourse/lib/ajax-error";
import i18n from "discourse-common/helpers/i18n";
import { bind } from "discourse-common/utils/decorators";
import AdminConfigAreaCard from "admin/components/admin-config-area-card";
import Permalink from "admin/models/permalink";
const TYPE_TO_FIELD_MAP = {
topic: "topicId",
post: "postId",
category: "categoryId",
tag: "tagName",
user: "userId",
external_url: "externalUrl",
};
export default class AdminFlagsForm extends Component {
@service router;
@service store;
@controller adminPermalinks;
get isUpdate() {
return this.args.permalink;
}
@cached
get formData() {
if (this.isUpdate) {
let permalinkType;
let permalinkValue;
if (!isEmpty(this.args.permalink.topic_id)) {
permalinkType = "topic";
permalinkValue = this.args.permalink.topic_id;
} else if (!isEmpty(this.args.permalink.post_id)) {
permalinkType = "post";
permalinkValue = this.args.permalink.post_id;
} else if (!isEmpty(this.args.permalink.category_id)) {
permalinkType = "category";
permalinkValue = this.args.permalink.category_id;
} else if (!isEmpty(this.args.permalink.tag_name)) {
permalinkType = "tag";
permalinkValue = this.args.permalink.tag_name;
} else if (!isEmpty(this.args.permalink.external_url)) {
permalinkType = "external_url";
permalinkValue = this.args.permalink.external_url;
} else if (!isEmpty(this.args.permalink.user_id)) {
permalinkType = "user";
permalinkValue = this.args.permalink.user_id;
}
return {
url: this.args.permalink.url,
[TYPE_TO_FIELD_MAP[permalinkType]]: permalinkValue,
permalinkType,
};
} else {
return {
permalinkType: "topic",
};
}
}
get header() {
return this.isUpdate
? "admin.permalink.form.edit_header"
: "admin.permalink.form.add_header";
}
@action
async save(data) {
this.isUpdate ? await this.update(data) : await this.create(data);
}
@bind
async create(data) {
try {
const result = await this.store.createRecord("permalink").save({
url: data.url,
permalink_type: data.permalinkType,
permalink_type_value: this.valueForPermalinkType(data),
});
this.adminPermalinks.model.unshift(Permalink.create(result.payload));
this.router.transitionTo("adminPermalinks");
} catch (error) {
popupAjaxError(error);
}
}
@bind
async update(data) {
try {
const result = await this.store.update(
"permalink",
this.args.permalink.id,
{
url: data.url,
permalink_type: data.permalinkType,
permalink_type_value: this.valueForPermalinkType(data),
}
);
const index = this.adminPermalinks.model.findIndex(
(permalink) => permalink.id === this.args.permalink.id
);
this.adminPermalinks.model[index] = Permalink.create(result.payload);
this.router.transitionTo("adminPermalinks");
} catch (error) {
popupAjaxError(error);
}
}
valueForPermalinkType(data) {
return data[TYPE_TO_FIELD_MAP[data.permalinkType]];
}
<template>
<BackButton @route="adminPermalinks" @label="admin.permalink.back" />
<div class="admin-config-area">
<div class="admin-config-area__primary-content admin-permalink-form">
<AdminConfigAreaCard @heading={{this.header}}>
<:content>
<Form
@onSubmit={{this.save}}
@data={{this.formData}}
as |form transientData|
>
<form.Field
@name="url"
@title={{i18n "admin.permalink.form.url"}}
@validation="required"
@format="large"
as |field|
>
<field.Input />
</form.Field>
<form.Field
@name="permalinkType"
@title={{i18n "admin.permalink.form.permalink_type"}}
@validation="required"
as |field|
>
<field.Select as |select|>
<select.Option @value="topic">{{i18n
"admin.permalink.topic_title"
}}</select.Option>
<select.Option @value="post">{{i18n
"admin.permalink.post_title"
}}</select.Option>
<select.Option @value="category">{{i18n
"admin.permalink.category_title"
}}</select.Option>
<select.Option @value="tag">{{i18n
"admin.permalink.tag_title"
}}</select.Option>
<select.Option @value="external_url">{{i18n
"admin.permalink.external_url"
}}</select.Option>
<select.Option @value="user">{{i18n
"admin.permalink.user_title"
}}</select.Option>
</field.Select>
</form.Field>
{{#if (eq transientData.permalinkType "topic")}}
<form.Field
@name="topicId"
@title={{i18n "admin.permalink.topic_id"}}
@format="small"
@validation="required"
as |field|
>
<field.Input />
</form.Field>
{{/if}}
{{#if (eq transientData.permalinkType "post")}}
<form.Field
@name="postId"
@title={{i18n "admin.permalink.post_id"}}
@format="small"
@validation="required"
as |field|
>
<field.Input />
</form.Field>
{{/if}}
{{#if (eq transientData.permalinkType "category")}}
<form.Field
@name="categoryId"
@title={{i18n "admin.permalink.category_id"}}
@format="small"
@validation="required"
as |field|
>
<field.Input />
</form.Field>
{{/if}}
{{#if (eq transientData.permalinkType "tag")}}
<form.Field
@name="tagName"
@title={{i18n "admin.permalink.tag_name"}}
@format="small"
@validation="required"
as |field|
>
<field.Input />
</form.Field>
{{/if}}
{{#if (eq transientData.permalinkType "external_url")}}
<form.Field
@name="externalUrl"
@title={{i18n "admin.permalink.external_url"}}
@format="large"
@validation="required"
as |field|
>
<field.Input />
</form.Field>
{{/if}}
{{#if (eq transientData.permalinkType "user")}}
<form.Field
@name="userId"
@title={{i18n "admin.permalink.user_id"}}
@format="small"
@validation="required"
as |field|
>
<field.Input />
</form.Field>
{{/if}}
<form.Submit @label="admin.permalink.form.save" />
</Form>
</:content>
</AdminConfigAreaCard>
</div>
</div>
</template>
}

View File

@ -9,8 +9,9 @@ import discourseDebounce from "discourse-common/lib/debounce";
import I18n from "discourse-i18n";
import Permalink from "admin/models/permalink";
export default class AdminPermalinksController extends Controller {
export default class AdminPermalinksIndexController extends Controller {
@service dialog;
@service toasts;
loading = false;
filter = null;
@ -29,34 +30,29 @@ export default class AdminPermalinksController extends Controller {
discourseDebounce(this, this._debouncedShow, INPUT_DELAY);
}
@action
recordAdded(arg) {
this.model.unshiftObject(arg);
}
@action
copyUrl(pl) {
let linkElement = document.querySelector(`#admin-permalink-${pl.id}`);
clipboardCopy(linkElement.textContent);
this.toasts.success({
duration: 3000,
data: {
message: I18n.t("admin.permalink.copy_success"),
},
});
}
@action
destroyRecord(record) {
return this.dialog.yesNoConfirm({
destroyRecord(permalink) {
this.dialog.yesNoConfirm({
message: I18n.t("admin.permalink.delete_confirm"),
didConfirm: () => {
return record.destroy().then(
(deleted) => {
if (deleted) {
this.model.removeObject(record);
} else {
this.dialog.alert(I18n.t("generic_error"));
}
},
function () {
this.dialog.alert(I18n.t("generic_error"));
}
);
didConfirm: async () => {
try {
await this.store.destroyRecord("permalink", permalink);
this.model.removeObject(permalink);
} catch {
this.dialog.alert(I18n.t("generic_error"));
}
},
});
}

View File

@ -1,10 +1,10 @@
import EmberObject from "@ember/object";
import { ajax } from "discourse/lib/ajax";
import DiscourseURL from "discourse/lib/url";
import Category from "discourse/models/category";
import RestModel from "discourse/models/rest";
import discourseComputed from "discourse-common/utils/decorators";
export default class Permalink extends EmberObject {
export default class Permalink extends RestModel {
static findAll(filter) {
return ajax("/admin/permalinks.json", { data: { filter } }).then(function (
permalinks
@ -13,17 +13,6 @@ export default class Permalink extends EmberObject {
});
}
save() {
return ajax("/admin/permalinks.json", {
type: "POST",
data: {
url: this.url,
permalink_type: this.permalink_type,
permalink_type_value: this.permalink_type_value,
},
});
}
@discourseComputed("category_id")
category(category_id) {
return Category.findById(category_id);
@ -34,9 +23,8 @@ export default class Permalink extends EmberObject {
return !DiscourseURL.isInternal(external_url);
}
destroy() {
return ajax("/admin/permalinks/" + this.id + ".json", {
type: "DELETE",
});
@discourseComputed("url")
key(url) {
return url.replace("/", "_");
}
}

View File

@ -0,0 +1,10 @@
import { service } from "@ember/service";
import DiscourseRoute from "discourse/routes/discourse";
export default class AdminPermalinksEditRoute extends DiscourseRoute {
@service store;
model(params) {
return this.store.find("permalink", params.permalink_id);
}
}

View File

@ -81,10 +81,14 @@ export default function () {
this.route("settings");
}
);
this.route("adminPermalinks", {
path: "/permalinks",
resetNamespace: true,
});
this.route(
"adminPermalinks",
{ path: "/permalinks", resetNamespace: true },
function () {
this.route("new");
this.route("edit", { path: "/:permalink_id" });
}
);
this.route("adminEmbedding", {
path: "/embedding",
resetNamespace: true,

View File

@ -0,0 +1 @@
<AdminPermalinkForm @permalink={{this.model}} />

View File

@ -0,0 +1,135 @@
<div class="admin-permalinks admin-config-page">
<AdminPageHeader
@titleLabel="admin.permalink.title"
@descriptionLabel="admin.permalink.description"
@learnMoreUrl="https://meta.discourse.org/t/redirect-old-forum-urls-to-new-discourse-urls/20930"
>
<:breadcrumbs>
<DBreadcrumbsItem
@path="/admin/customize/permalinks"
@label={{i18n "admin.permalink.title"}}
/>
</:breadcrumbs>
<:actions as |actions|>
<actions.Primary
@route="adminPermalinks.new"
@title="admin.permalink.add"
@label="admin.permalink.add"
@icon="plus"
@disabled={{this.addFlagButtonDisabled}}
class="admin-permalinks__header-add-permalink"
/>
</:actions>
</AdminPageHeader>
<div class="admin-container admin-config-page__main-area">
<ConditionalLoadingSpinner @condition={{this.loading}}>
<div class="permalink-search">
<TextField
@value={{this.filter}}
@placeholderKey="admin.permalink.form.filter"
@autocorrect="off"
@autocapitalize="off"
class="url-input"
/>
</div>
<div class="permalink-results">
{{#if this.model.length}}
<table class="d-admin-table permalinks">
<thead>
<th>{{i18n "admin.permalink.url"}}</th>
<th>{{i18n "admin.permalink.destination"}}</th>
</thead>
<tbody>
{{#each this.model as |pl|}}
<tr
class={{concat-class
"admin-permalink-item d-admin-row__content"
pl.key
}}
>
<td>
<FlatButton
@title="admin.permalink.copy_to_clipboard"
@icon="far-clipboard"
@action={{fn this.copyUrl pl}}
/>
<span
id="admin-permalink-{{pl.id}}"
class="admin-permalink-item__url"
title={{pl.url}}
>{{pl.url}}</span>
</td>
<td class="destination">
{{#if pl.topic_id}}
<a href={{pl.topic_url}}>{{pl.topic_title}}</a>
{{/if}}
{{#if pl.post_id}}
<a href={{pl.post_url}}>{{pl.post_topic_title}}
#{{pl.post_number}}</a>
{{/if}}
{{#if pl.category_id}}
{{category-link pl.category}}
{{/if}}
{{#if pl.tag_id}}
<a href={{pl.tag_url}}>{{pl.tag_name}}</a>
{{/if}}
{{#if pl.external_url}}
{{#if pl.linkIsExternal}}
{{d-icon "up-right-from-square"}}
{{/if}}
<a href={{pl.external_url}}>{{pl.external_url}}</a>
{{/if}}
{{#if pl.user_id}}
<a href={{pl.user_url}}>{{pl.username}}</a>
{{/if}}
</td>
<td class="d-admin-row__controls">
<div class="d-admin-row__controls-options">
<DButton
class="btn-small admin-permalink-item__edit"
@route="adminPermalinks.edit"
@routeModels={{pl}}
@label="admin.config_areas.flags.edit"
/>
<DMenu
@identifier="permalink-menu"
@title={{i18n "admin.permalinks.more_options"}}
@icon="ellipsis-vertical"
@onRegisterApi={{this.onRegisterApi}}
>
<:content>
<DropdownMenu as |dropdown|>
<dropdown.item>
<DButton
@action={{fn this.destroyRecord pl}}
@icon="trash-can"
class="btn-transparent admin-permalink-item__delete"
@label="admin.config_areas.flags.delete"
/>
</dropdown.item>
</DropdownMenu>
</:content>
</DMenu>
</div>
</td>
</tr>
{{/each}}
</tbody>
</table>
{{else}}
{{#if this.filter}}
<p class="permalink-results__no-result">{{i18n
"search.no_results"
}}</p>
{{else}}
<p class="permalink-results__no-permalinks">{{i18n
"admin.permalink.no_permalinks"
}}</p>
{{/if}}
{{/if}}
</div>
</ConditionalLoadingSpinner>
</div>
</div>

View File

@ -0,0 +1 @@
<AdminPermalinkForm />

View File

@ -1,88 +0,0 @@
<h1>{{i18n "admin.permalink.title"}}</h1>
<div class="permalink-description">
<span>{{i18n "admin.permalink.description"}}</span>
</div>
<PermalinkForm @action={{action "recordAdded"}} />
<ConditionalLoadingSpinner @condition={{this.loading}}>
<div class="permalink-search">
<TextField
@value={{this.filter}}
@placeholderKey="admin.permalink.form.filter"
@autocorrect="off"
@autocapitalize="off"
class="url-input"
/>
</div>
<div class="permalink-results">
{{#if this.model.length}}
<table class="admin-logs-table permalinks grid">
<thead class="heading-container">
<th class="col heading first url">{{i18n "admin.permalink.url"}}</th>
<th class="col heading destination">{{i18n
"admin.permalink.destination"
}}</th>
<th class="col heading actions"></th>
</thead>
<tbody>
{{#each this.model as |pl|}}
<tr class="admin-list-item">
<td class="col first url">
<FlatButton
@title="admin.permalink.copy_to_clipboard"
@icon="far-clipboard"
@action={{action "copyUrl" pl}}
/>
<span
id="admin-permalink-{{pl.id}}"
title={{pl.url}}
>{{pl.url}}</span>
</td>
<td class="col destination">
{{#if pl.topic_id}}
<a href={{pl.topic_url}}>{{pl.topic_title}}</a>
{{/if}}
{{#if pl.post_id}}
<a href={{pl.post_url}}>{{pl.post_topic_title}}
#{{pl.post_number}}</a>
{{/if}}
{{#if pl.category_id}}
{{category-link pl.category}}
{{/if}}
{{#if pl.tag_id}}
<a href={{pl.tag_url}}>{{pl.tag_name}}</a>
{{/if}}
{{#if pl.external_url}}
{{#if pl.linkIsExternal}}
{{d-icon "up-right-from-square"}}
{{/if}}
<a href={{pl.external_url}}>{{pl.external_url}}</a>
{{/if}}
{{#if pl.user_id}}
<a href={{pl.user_url}}>{{pl.username}}</a>
{{/if}}
</td>
<td class="col action" style="text-align: right;">
<DButton
@action={{fn this.destroyRecord pl}}
@icon="trash-can"
class="btn-danger"
/>
</td>
</tr>
{{/each}}
</tbody>
</table>
{{else}}
{{#if this.filter}}
<p class="permalink-results__no-result">{{i18n "search.no_results"}}</p>
{{else}}
<p class="permalink-results__no-permalinks">{{i18n
"admin.permalink.no_permalinks"
}}</p>
{{/if}}
{{/if}}
</div>
</ConditionalLoadingSpinner>

View File

@ -1,11 +1,15 @@
import { eq } from "truth-helpers";
import { notEq } from "truth-helpers";
import concatClass from "discourse/helpers/concat-class";
const FKControlConditionalContentItem = <template>
{{#if (eq @name @activeName)}}
<div class="form-kit__conditional-display-content">
{{yield}}
</div>
{{/if}}
<div
class={{concatClass
"form-kit__conditional-display-content"
(if (notEq @name @activeName) "hidden")
}}
>
{{yield}}
</div>
</template>;
export default FKControlConditionalContentItem;

View File

@ -1,17 +1,15 @@
import Component from "@glimmer/component";
import { tracked } from "@glimmer/tracking";
import { concat } from "@ember/helper";
import { action } from "@ember/object";
import { isBlank } from "@ember/utils";
import UppyImageUploader from "discourse/components/uppy-image-uploader";
export default class FKControlImage extends Component {
static controlType = "image";
@tracked imageUrl = this.args.value;
@action
setImage(upload) {
this.args.field.set(upload);
this.imageUrl = upload?.url;
}
@action
@ -19,6 +17,10 @@ export default class FKControlImage extends Component {
this.setImage(undefined);
}
get imageUrl() {
return isBlank(this.args.value) ? null : this.args.value;
}
<template>
<UppyImageUploader
@id={{concat @field.id "-" @field.name}}

View File

@ -160,6 +160,7 @@ class FKForm extends Component {
@action
unregisterField(name) {
this.fields.delete(name);
this.removeError(name);
}
@action

View File

@ -1,4 +1,5 @@
import { capitalize } from "@ember/string";
import { isBlank } from "@ember/utils";
import QUnit from "qunit";
import { query } from "discourse/tests/helpers/qunit-helpers";
@ -50,7 +51,7 @@ class FieldHelper {
case "image": {
return this.element
.querySelector(".form-kit__control-image a.lightbox")
.getAttribute("href");
?.getAttribute("href");
}
case "radio-group": {
return this.element.querySelector(".form-kit__control-radio:checked")
@ -114,6 +115,10 @@ class FieldHelper {
this.context.deepEqual(this.value, value, message);
}
hasNoValue(message) {
this.context.true(isBlank(this.value), message);
}
isDisabled(message) {
this.context.ok(this.disabled, message);
}

View File

@ -1,4 +1,5 @@
import { render } from "@ember/test-helpers";
import { fn } from "@ember/helper";
import { click, render } from "@ember/test-helpers";
import { module, test, todo } from "qunit";
import Form from "discourse/components/form";
import { setupRenderingTest } from "discourse/tests/helpers/component-test";
@ -34,7 +35,7 @@ module(
let data = { image_url: "/images/discourse-logo-sketch-small.png" };
await render(<template>
<Form @mutable={{true}} @data={{data}} as |form|>
<Form @data={{data}} as |form|>
<form.Field @name="image_url" @title="Foo" as |field|>
<field.Image @type="site_setting" />
</form.Field>
@ -46,6 +47,28 @@ module(
assert.form().field("image_url").hasValue(data.image_url);
});
test("removing the upload", async function (assert) {
let data = { image_url: "/images/discourse-logo-sketch-small.png" };
await render(<template>
<Form @data={{data}} as |form|>
<form.Field @name="image_url" @title="Foo" as |field|>
<field.Image @type="site_setting" />
</form.Field>
<form.Button class="test" @action={{fn form.set "image_url" null}} />
</Form>
</template>);
await formKit().submit();
assert.form().field("image_url").hasValue(data.image_url);
await click(".test");
assert.form().field("image_url").hasNoValue();
});
todo("when disabled", async function () {});
}
);

View File

@ -264,4 +264,34 @@ module("Integration | Component | FormKit | Form", function (hooks) {
assert.form().hasNoErrors();
});
test("destroying field", async function (assert) {
await render(<template>
<Form @data={{hash visible=true}} as |form data|>
{{#if data.visible}}
<form.Field
@title="Foo"
@name="foo"
@validation="required"
as |field|
>
<field.Input />
</form.Field>
{{/if}}
<form.Button
class="test"
@action={{fn form.setProperties (hash visible=false)}}
/>
</Form>
</template>);
await formKit().submit();
assert.form().hasErrors({ foo: "Required" });
await click(".test");
assert.form().hasNoErrors("remove the errors associated with this field");
});
});

View File

@ -797,49 +797,36 @@
color: var(--primary-medium);
}
// Permalinks
.permalinks {
.url,
.topic,
.category,
.external_url,
.destination,
.post {
@include ellipsis;
max-width: 100px;
@include breakpoint(tablet) {
max-width: 100%;
}
}
&.grid tr.admin-list-item {
grid-template-columns: unset;
}
}
.permalink-form {
padding: 0.5em 1em 0 1em;
margin-top: 1em;
background: var(--primary-very-low);
.select-kit {
max-width: 260px;
}
.admin-permalinks {
@include breakpoint(tablet) {
label {
.admin-page-subheader,
.admin-config-area,
.admin-config-area__primary-content,
.loading-container {
width: 100%;
}
.destination {
margin-top: 0.5em;
}
.d-admin-row__controls-options {
padding-bottom: 1em;
}
td {
width: auto;
}
}
.permalink-search input {
width: 100%;
}
}
.permalink-description {
color: var(--primary-medium);
}
.permalink-search {
margin-top: 2em;
input {
min-width: 250px;
margin-bottom: 0;
.admin-permalink-item {
&__delete.btn,
&__delete.btn:hover {
border-top: 1px solid var(--primary-low);
color: var(--danger);
svg {
color: var(--danger);
}
}
}

View File

@ -1,7 +1,7 @@
# frozen_string_literal: true
class Admin::PermalinksController < Admin::AdminController
before_action :fetch_permalink, only: [:destroy]
before_action :fetch_permalink, only: %i[show update destroy]
def index
url = params[:filter]
@ -9,23 +9,38 @@ class Admin::PermalinksController < Admin::AdminController
render_serialized(permalinks, PermalinkSerializer)
end
def new
end
def edit
end
def show
render_serialized(@permalink, PermalinkSerializer)
end
def create
params.require(:url)
params.require(:permalink_type)
params.require(:permalink_type_value)
if params[:permalink_type] == "tag_name"
params[:permalink_type] = "tag_id"
params[:permalink_type_value] = Tag.find_by_name(params[:permalink_type_value])&.id
end
permalink =
Permalink.new(:url => params[:url], params[:permalink_type] => params[:permalink_type_value])
if permalink.save
render_serialized(permalink, PermalinkSerializer)
else
render_json_error(permalink)
end
Permalink.create!(
url: permalink_params[:url],
permalink_type: permalink_params[:permalink_type],
permalink_type_value: permalink_params[:permalink_type_value],
)
render_serialized(permalink, PermalinkSerializer)
rescue ActiveRecord::RecordInvalid => e
render_json_error(e.record.errors.full_messages)
end
def update
@permalink.update!(
url: permalink_params[:url],
permalink_type: permalink_params[:permalink_type],
permalink_type_value: permalink_params[:permalink_type_value],
)
render_serialized(@permalink, PermalinkSerializer)
rescue ActiveRecord::RecordInvalid => e
render_json_error(e.record.errors.full_messages)
end
def destroy
@ -38,4 +53,8 @@ class Admin::PermalinksController < Admin::AdminController
def fetch_permalink
@permalink = Permalink.find(params[:id])
end
def permalink_params
params.require(:permalink).permit(:url, :permalink_type, :permalink_type_value)
end
end

View File

@ -1,16 +1,31 @@
# frozen_string_literal: true
class Permalink < ActiveRecord::Base
attr_accessor :permalink_type, :permalink_type_value
belongs_to :topic
belongs_to :post
belongs_to :category
belongs_to :tag
belongs_to :user
before_validation :clear_associations
before_validation :normalize_url, :encode_url
before_validation :set_association_value
validates :url, uniqueness: true
validates :topic_id, presence: true, if: Proc.new { |permalink| permalink.topic_type? }
validates :post_id, presence: true, if: Proc.new { |permalink| permalink.post_type? }
validates :category_id, presence: true, if: Proc.new { |permalink| permalink.category_type? }
validates :tag_id, presence: true, if: Proc.new { |permalink| permalink.tag_type? }
validates :user_id, presence: true, if: Proc.new { |permalink| permalink.user_type? }
validates :external_url, presence: true, if: Proc.new { |permalink| permalink.external_url_type? }
%i[topic post category tag user external_url].each do |association|
define_method("#{association}_type?") { self.permalink_type == association.to_s }
end
class Normalizer
attr_reader :source
@ -98,6 +113,24 @@ class Permalink < ActiveRecord::Base
def relative_external_url
external_url.match?(%r{\A/[^/]}) ? "#{Discourse.base_path}#{external_url}" : external_url
end
def clear_associations
self.topic_id = nil
self.post_id = nil
self.category_id = nil
self.user_id = nil
self.tag_id = nil
self.external_url = nil
end
def set_association_value
self.topic_id = self.permalink_type_value if self.topic_type?
self.post_id = self.permalink_type_value if self.post_type?
self.user_id = self.permalink_type_value if self.user_type?
self.category_id = self.permalink_type_value if self.category_type?
self.external_url = self.permalink_type_value if self.external_url_type?
self.tag_id = Tag.where(name: self.permalink_type_value).first&.id if self.tag_type?
end
end
# == Schema Information

View File

@ -7271,17 +7271,27 @@ en:
category_id: "Category ID"
category_title: "Category"
tag_name: "Tag name"
tag_title: "Tag"
external_url: "External or Relative URL"
user_id: "User ID"
user_title: "User"
username: "Username"
destination: "Destination"
copy_to_clipboard: "Copy Permalink to Clipboard"
delete_confirm: Are you sure you want to delete this permalink?
no_permalinks: "You don't have any permalinks yet. Create a new permalink above to begin seeing a list of your permalinks here."
add: "Add permalink"
back: "Back to Permalinks"
more_options: "More options"
copy_success: "Permalink copied to clipboard"
form:
label: "New:"
add: "Add"
filter: "Search (URL or External URL)"
add_header: "Add permalink"
edit_header: "Edit permalink"
filter: "Search URL or destination URL"
url: "URL"
permalink_type: "Permalink type"
save: "Save"
reseed:
action:

View File

@ -300,13 +300,17 @@ Discourse::Application.routes.draw do
resource :email_style, only: %i[show update]
get "email_style/:field" => "email_styles#show", :constraints => { field: /html|css/ }
resources :permalinks, only: %i[index new create show destroy]
end
resources :embeddable_hosts, only: %i[create update destroy], constraints: AdminConstraint.new
resources :color_schemes,
only: %i[index create update destroy],
constraints: AdminConstraint.new
resources :permalinks, only: %i[index create destroy], constraints: AdminConstraint.new
resources :permalinks,
only: %i[index create show update destroy],
constraints: AdminConstraint.new
scope "/customize" do
resources :watched_words, only: %i[index create destroy] do

View File

@ -33,6 +33,61 @@ RSpec.describe Permalink do
expect(permalink.errors[:url]).to be_present
end
it "validates association" do
permalink = described_class.create(url: "/my/old/url", permalink_type: "topic")
expect(permalink.errors[:topic_id]).to be_present
permalink = described_class.create(url: "/my/old/url", permalink_type: "post")
expect(permalink.errors[:post_id]).to be_present
permalink = described_class.create(url: "/my/old/url", permalink_type: "category")
expect(permalink.errors[:category_id]).to be_present
permalink = described_class.create(url: "/my/old/url", permalink_type: "user")
expect(permalink.errors[:user_id]).to be_present
permalink = described_class.create(url: "/my/old/url", permalink_type: "external_url")
expect(permalink.errors[:external_url]).to be_present
permalink = described_class.create(url: "/my/old/url", permalink_type: "tag")
expect(permalink.errors[:tag_id]).to be_present
end
it "clears associations when permalink_type changes" do
permalink = described_class.create!(url: " my/old/url ")
permalink.update!(permalink_type_value: 1, permalink_type: "topic")
expect(permalink.topic_id).to eq(1)
permalink.update!(permalink_type_value: 1, permalink_type: "post")
expect(permalink.topic_id).to be_nil
expect(permalink.post_id).to eq(1)
permalink.update!(permalink_type_value: 1, permalink_type: "category")
expect(permalink.post_id).to be_nil
expect(permalink.category_id).to eq(1)
permalink.update!(permalink_type_value: 1, permalink_type: "user")
expect(permalink.category_id).to be_nil
expect(permalink.user_id).to eq(1)
permalink.update!(
permalink_type_value: "https://discourse.org",
permalink_type: "external_url",
)
expect(permalink.user_id).to be_nil
expect(permalink.external_url).to eq("https://discourse.org")
tag = Fabricate(:tag, name: "art")
permalink.update!(permalink_type_value: "art", permalink_type: "tag")
expect(permalink.external_url).to be_nil
expect(permalink.tag_id).to eq(tag.id)
permalink.update!(permalink_type_value: 1, permalink_type: "topic")
expect(permalink.tag_id).to be_nil
expect(permalink.topic_id).to eq(1)
end
context "with special characters in URL" do
it "percent encodes any special character" do
permalink = described_class.create!(url: "/2022/10/03/привет-sam")

View File

@ -10,10 +10,20 @@ RSpec.describe Admin::PermalinksController do
before { sign_in(admin) }
it "filters url" do
Fabricate(:permalink, url: "/forum/23")
Fabricate(:permalink, url: "/forum/98")
Fabricate(:permalink, url: "/discuss/topic/45")
Fabricate(:permalink, url: "/discuss/topic/76")
Fabricate(:permalink, url: "/forum/23", permalink_type_value: "1", permalink_type: "topic")
Fabricate(:permalink, url: "/forum/98", permalink_type_value: "1", permalink_type: "topic")
Fabricate(
:permalink,
url: "/discuss/topic/45",
permalink_type_value: "1",
permalink_type: "topic",
)
Fabricate(
:permalink,
url: "/discuss/topic/76",
permalink_type_value: "1",
permalink_type: "topic",
)
get "/admin/permalinks.json", params: { filter: "topic" }
@ -23,10 +33,26 @@ RSpec.describe Admin::PermalinksController do
end
it "filters external url" do
Fabricate(:permalink, external_url: "http://google.com")
Fabricate(:permalink, external_url: "http://wikipedia.org")
Fabricate(:permalink, external_url: "http://www.discourse.org")
Fabricate(:permalink, external_url: "http://try.discourse.org")
Fabricate(
:permalink,
permalink_type_value: "http://google.com",
permalink_type: "external_url",
)
Fabricate(
:permalink,
permalink_type_value: "http://wikipedia.org",
permalink_type: "external_url",
)
Fabricate(
:permalink,
permalink_type_value: "http://www.discourse.org",
permalink_type: "external_url",
)
Fabricate(
:permalink,
permalink_type_value: "http://try.discourse.org",
permalink_type: "external_url",
)
get "/admin/permalinks.json", params: { filter: "discourse" }
@ -36,10 +62,30 @@ RSpec.describe Admin::PermalinksController do
end
it "filters url and external url both" do
Fabricate(:permalink, url: "/forum/23", external_url: "http://google.com")
Fabricate(:permalink, url: "/discourse/98", external_url: "http://wikipedia.org")
Fabricate(:permalink, url: "/discuss/topic/45", external_url: "http://discourse.org")
Fabricate(:permalink, url: "/discuss/topic/76", external_url: "http://try.discourse.org")
Fabricate(
:permalink,
url: "/forum/23",
permalink_type_value: "http://google.com",
permalink_type: "external_url",
)
Fabricate(
:permalink,
url: "/discourse/98",
permalink_type_value: "http://wikipedia.org",
permalink_type: "external_url",
)
Fabricate(
:permalink,
url: "/discuss/topic/45",
permalink_type_value: "http://discourse.org",
permalink_type: "external_url",
)
Fabricate(
:permalink,
url: "/discuss/topic/76",
permalink_type_value: "http://try.discourse.org",
permalink_type: "external_url",
)
get "/admin/permalinks.json", params: { filter: "discourse" }
@ -80,9 +126,11 @@ RSpec.describe Admin::PermalinksController do
post "/admin/permalinks.json",
params: {
url: "/topics/771",
permalink_type: "topic_id",
permalink_type_value: topic.id,
permalink: {
url: "/topics/771",
permalink_type: "topic",
permalink_type_value: topic.id,
},
}
expect(response.status).to eq(200)
@ -102,9 +150,11 @@ RSpec.describe Admin::PermalinksController do
post "/admin/permalinks.json",
params: {
url: "/topics/771/8291",
permalink_type: "post_id",
permalink_type_value: some_post.id,
permalink: {
url: "/topics/771/8291",
permalink_type: "post",
permalink_type_value: some_post.id,
},
}
expect(response.status).to eq(200)
@ -124,9 +174,11 @@ RSpec.describe Admin::PermalinksController do
post "/admin/permalinks.json",
params: {
url: "/forums/11",
permalink_type: "category_id",
permalink_type_value: category.id,
permalink: {
url: "/forums/11",
permalink_type: "category",
permalink_type_value: category.id,
},
}
expect(response.status).to eq(200)
@ -146,9 +198,11 @@ RSpec.describe Admin::PermalinksController do
post "/admin/permalinks.json",
params: {
url: "/forums/12",
permalink_type: "tag_name",
permalink_type_value: tag.name,
permalink: {
url: "/forums/12",
permalink_type: "tag",
permalink_type_value: tag.name,
},
}
expect(response.status).to eq(200)
@ -168,9 +222,11 @@ RSpec.describe Admin::PermalinksController do
post "/admin/permalinks.json",
params: {
url: "/people/42",
permalink_type: "user_id",
permalink_type_value: user.id,
permalink: {
url: "/people/42",
permalink_type: "user",
permalink_type_value: user.id,
},
}
expect(response.status).to eq(200)
@ -193,9 +249,11 @@ RSpec.describe Admin::PermalinksController do
expect do
post "/admin/permalinks.json",
params: {
url: "/topics/771",
permalink_type: "topic_id",
permalink_type_value: topic.id,
permalink: {
url: "/topics/771",
permalink_type: "topic",
permalink_type_value: topic.id,
},
}
end.not_to change { Permalink.count }

View File

@ -345,7 +345,11 @@ RSpec.describe ApplicationController do
describe "topic not found" do
it "should not redirect to permalink if topic/category does not exist" do
topic = create_post.topic
Permalink.create!(url: topic.relative_url, topic_id: topic.id + 1)
Permalink.create!(
url: topic.relative_url,
permalink_type_value: topic.id + 1,
permalink_type: "topic",
)
topic.trash!
SiteSetting.detailed_404 = false
@ -360,7 +364,11 @@ RSpec.describe ApplicationController do
it "should return permalink for deleted topics" do
topic = create_post.topic
external_url = "https://somewhere.over.rainbow"
Permalink.create!(url: topic.relative_url, external_url: external_url)
Permalink.create!(
url: topic.relative_url,
permalink_type_value: external_url,
permalink_type: "external_url",
)
topic.trash!
get topic.relative_url
@ -382,7 +390,12 @@ RSpec.describe ApplicationController do
trashed_topic = create_post.topic
trashed_topic.trash!
new_topic = create_post.topic
permalink = Permalink.create!(url: trashed_topic.relative_url, topic_id: new_topic.id)
permalink =
Permalink.create!(
url: trashed_topic.relative_url,
permalink_type_value: new_topic.id,
permalink_type: "topic",
)
# no subfolder because router doesn't know about subfolder in this test
get "/t/#{trashed_topic.slug}/#{trashed_topic.id}"
@ -391,14 +404,23 @@ RSpec.describe ApplicationController do
permalink.destroy
category = Fabricate(:category)
permalink = Permalink.create!(url: trashed_topic.relative_url, category_id: category.id)
permalink =
Permalink.create!(
url: trashed_topic.relative_url,
permalink_type_value: category.id,
permalink_type: "category",
)
get "/t/#{trashed_topic.slug}/#{trashed_topic.id}"
expect(response.status).to eq(301)
expect(response).to redirect_to("/forum/c/#{category.slug}/#{category.id}")
permalink.destroy
permalink =
Permalink.create!(url: trashed_topic.relative_url, post_id: new_topic.posts.last.id)
Permalink.create!(
url: trashed_topic.relative_url,
permalink_type_value: new_topic.posts.last.id,
permalink_type: "post",
)
get "/t/#{trashed_topic.slug}/#{trashed_topic.id}"
expect(response.status).to eq(301)
expect(response).to redirect_to(

View File

@ -44,7 +44,12 @@ RSpec.describe CategoriesController do
end
it "respects permalinks before redirecting /category paths to /c paths" do
_perm = Permalink.create!(url: "category/something", category_id: category.id)
_perm =
Permalink.create!(
url: "category/something",
permalink_type_value: category.id,
permalink_type: "category",
)
get "/category/something"
expect(response).to have_http_status(:moved_permanently)

View File

@ -6,7 +6,7 @@ RSpec.describe PermalinksController do
describe "show" do
it "should redirect to a permalink's target_url with status 301" do
permalink.update!(topic_id: topic.id)
permalink.update!(permalink_type_value: topic.id, permalink_type: "topic")
get "/#{permalink.url}"
@ -15,7 +15,7 @@ RSpec.describe PermalinksController do
end
it "should work for subfolder installs too" do
permalink.update!(topic_id: topic.id)
permalink.update!(permalink_type_value: topic.id, permalink_type: "topic")
set_subfolder "/forum"
get "/#{permalink.url}"
@ -25,7 +25,7 @@ RSpec.describe PermalinksController do
end
it "should apply normalizations" do
permalink.update!(external_url: "/topic/100")
permalink.update!(permalink_type_value: "/topic/100", permalink_type: "external_url")
SiteSetting.permalink_normalizations = "/(.*)\\?.*/\\1"
get "/#{permalink.url}", params: { test: "hello" }
@ -46,7 +46,12 @@ RSpec.describe PermalinksController do
end
context "when permalink's target_url is an external URL" do
before { permalink.update!(external_url: "https://github.com/discourse/discourse") }
before do
permalink.update!(
permalink_type_value: "https://github.com/discourse/discourse",
permalink_type: "external_url",
)
end
it "redirects to it properly" do
get "/#{permalink.url}"

View File

@ -0,0 +1,30 @@
# frozen_string_literal: true
describe "Admin Permalinks Page", type: :system do
fab!(:admin)
fab!(:post)
let(:admin_permalinks_page) { PageObjects::Pages::AdminPermalinks.new }
let(:admin_permalink_form_page) { PageObjects::Pages::AdminPermalinkForm.new }
before { sign_in(admin) }
it "allows admin to create, edit, and destroy permalink" do
admin_permalinks_page.visit
admin_permalinks_page.click_add_permalink
admin_permalink_form_page
.fill_in_url("test")
.select_permalink_type("category")
.fill_in_category("1")
.click_save
expect(admin_permalinks_page).to have_permalinks("test")
admin_permalinks_page.click_edit_permalink("test")
admin_permalink_form_page.fill_in_url("test2").click_save
expect(admin_permalinks_page).to have_permalinks("test2")
admin_permalinks_page.click_delete_permalink("test2")
expect(admin_permalinks_page).to have_no_permalinks
end
end

View File

@ -0,0 +1,39 @@
# frozen_string_literal: true
module PageObjects
module Pages
class AdminPermalinkForm < PageObjects::Pages::Base
def fill_in_url(url)
form.field("url").fill_in(url)
self
end
def fill_in_description(description)
form.field("description").fill_in(description)
self
end
def select_permalink_type(type)
form.field("permalinkType").select(type)
self
end
def fill_in_category(category)
form.field("categoryId").fill_in(category)
self
end
def click_save
form.submit
expect(page).to have_css(
".admin-permalink-item__url",
wait: Capybara.default_max_wait_time * 3,
)
end
def form
@form ||= PageObjects::Components::FormKit.new(".admin-permalink-form .form-kit")
end
end
end
end

View File

@ -0,0 +1,54 @@
# frozen_string_literal: true
module PageObjects
module Pages
class AdminPermalinks < PageObjects::Pages::Base
def visit
page.visit("/admin/customize/permalinks")
self
end
def toggle(key)
PageObjects::Components::DToggleSwitch.new(".admin-flag-item__toggle.#{key}").toggle
has_saved_flag?(key)
self
end
def click_add_permalink
find(".admin-permalinks__header-add-permalink").click
self
end
def click_edit_permalink(url)
find("tr.#{url} .admin-permalink-item__edit").click
self
end
def click_delete_permalink(url)
open_permalink_menu(url)
find(".admin-permalink-item__delete").click
find(".dialog-footer .btn-primary").click
expect(page).to have_no_css(".dialog-body")
has_closed_permalink_menu?
self
end
def has_permalinks?(*permalinks)
all(".admin-permalink-item__url").map(&:text) == permalinks
end
def has_no_permalinks?
has_no_css?(".admin-permalink-item__url")
end
def open_permalink_menu(url)
find("tr.#{url} .permalink-menu-trigger").click
self
end
def has_closed_permalink_menu?
has_no_css?(".permalink-menu-content")
end
end
end
end