From 3266350e80da4508243f667a39fd9d07e031adf1 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Thu, 30 Jun 2022 14:54:20 +0800 Subject: [PATCH] FEATURE: Decouple category/tag presence in sidebar from notifi level (#17273) --- .../components/sidebar/categories-section.js | 39 +++- .../app/components/sidebar/tags-section.js | 37 ++- .../app/controllers/preferences/sidebar.js | 41 ++++ .../javascripts/discourse/app/models/user.js | 39 +++- .../discourse/app/routes/app-route-map.js | 1 + .../app/routes/preferences-sidebar.js | 20 ++ .../components/sidebar/categories-section.hbs | 7 +- .../components/sidebar/tags-section.hbs | 7 +- .../discourse/app/templates/preferences.hbs | 7 + .../app/templates/preferences/interface.hbs | 4 - .../app/templates/preferences/sidebar.hbs | 49 ++++ .../sidebar-categories-section-test.js | 133 ++++------- .../acceptance/sidebar-tags-section-test.js | 61 ++--- .../user-preferences-sidebar-test.js | 213 ++++++++++++++++++ .../tests/helpers/create-pretender.js | 6 +- .../tests/helpers/select-kit-helper.js | 4 + .../addon/components/category-selector.js | 10 +- .../common/select-kit/category-row.scss | 14 +- app/controllers/users_controller.rb | 18 +- app/models/category.rb | 1 + app/models/sidebar_section_link.rb | 35 +++ app/models/tag.rb | 1 + app/models/user.rb | 4 + app/serializers/current_user_serializer.rb | 20 +- app/services/user_updater.rb | 50 ++++ config/locales/client.en.yml | 17 +- config/locales/server.en.yml | 5 + config/routes.rb | 1 + ...0628031850_create_sidebar_section_links.rb | 15 ++ .../sidebar_section_link_fabricator.rb | 13 ++ spec/models/category_spec.rb | 11 + spec/models/sidebar_section_link_spec.rb | 39 ++++ spec/models/tag_spec.rb | 12 + spec/models/user_spec.rb | 9 + spec/requests/users_controller_spec.rb | 111 +++++++-- .../current_user_serializer_spec.rb | 91 +++++++- 36 files changed, 965 insertions(+), 180 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js create mode 100644 app/assets/javascripts/discourse/app/routes/preferences-sidebar.js create mode 100644 app/assets/javascripts/discourse/app/templates/preferences/sidebar.hbs create mode 100644 app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js create mode 100644 app/models/sidebar_section_link.rb create mode 100644 db/migrate/20220628031850_create_sidebar_section_links.rb create mode 100644 spec/fabricators/sidebar_section_link_fabricator.rb create mode 100644 spec/models/sidebar_section_link_spec.rb diff --git a/app/assets/javascripts/discourse/app/components/sidebar/categories-section.js b/app/assets/javascripts/discourse/app/components/sidebar/categories-section.js index f1f42406507..fa96b5eb359 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/categories-section.js +++ b/app/assets/javascripts/discourse/app/components/sidebar/categories-section.js @@ -1,9 +1,15 @@ +import I18n from "I18n"; + import { cached } from "@glimmer/tracking"; +import { inject as service } from "@ember/service"; +import { action } from "@ember/object"; import GlimmerComponent from "discourse/components/glimmer"; import CategorySectionLink from "discourse/lib/sidebar/categories-section/category-section-link"; export default class SidebarCategoriesSection extends GlimmerComponent { + @service router; + constructor() { super(...arguments); @@ -20,11 +26,32 @@ export default class SidebarCategoriesSection extends GlimmerComponent { @cached get sectionLinks() { - return this.site.trackedCategoriesList.map((trackedCategory) => { - return new CategorySectionLink({ - category: trackedCategory, - topicTrackingState: this.topicTrackingState, - }); - }); + const links = []; + + for (const category of this.currentUser.sidebarCategories) { + links.push( + new CategorySectionLink({ + category, + topicTrackingState: this.topicTrackingState, + }) + ); + } + + return links; + } + + get noCategoriesText() { + const url = `/u/${this.currentUser.username}/preferences/sidebar`; + + return `${I18n.t( + "sidebar.sections.categories.none" + )} ${I18n.t( + "sidebar.sections.categories.click_to_get_started" + )}`; + } + + @action + editTracked() { + this.router.transitionTo("preferences.sidebar", this.currentUser); } } diff --git a/app/assets/javascripts/discourse/app/components/sidebar/tags-section.js b/app/assets/javascripts/discourse/app/components/sidebar/tags-section.js index 97d085ae22d..47a845ceb23 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/tags-section.js +++ b/app/assets/javascripts/discourse/app/components/sidebar/tags-section.js @@ -1,9 +1,15 @@ +import I18n from "I18n"; + import { cached } from "@glimmer/tracking"; +import { inject as service } from "@ember/service"; +import { action } from "@ember/object"; import GlimmerComponent from "discourse/components/glimmer"; import TagSectionLink from "discourse/lib/sidebar/tags-section/tag-section-link"; export default class SidebarTagsSection extends GlimmerComponent { + @service router; + constructor() { super(...arguments); @@ -20,11 +26,30 @@ export default class SidebarTagsSection extends GlimmerComponent { @cached get sectionLinks() { - return this.currentUser.trackedTags.map((trackedTag) => { - return new TagSectionLink({ - tagName: trackedTag, - topicTrackingState: this.topicTrackingState, - }); - }); + const links = []; + + for (const tagName of this.currentUser.sidebarTagNames) { + links.push( + new TagSectionLink({ + tagName, + topicTrackingState: this.topicTrackingState, + }) + ); + } + + return links; + } + + get noTagsText() { + const url = `/u/${this.currentUser.username}/preferences/sidebar`; + + return `${I18n.t("sidebar.sections.tags.none")} ${I18n.t( + "sidebar.sections.tags.click_to_get_started" + )}`; + } + + @action + editTracked() { + this.router.transitionTo("preferences.sidebar", this.currentUser); } } diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js b/app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js new file mode 100644 index 00000000000..10358abf311 --- /dev/null +++ b/app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js @@ -0,0 +1,41 @@ +import Controller from "@ember/controller"; +import { action } from "@ember/object"; +import { tracked } from "@glimmer/tracking"; + +import { popupAjaxError } from "discourse/lib/ajax-error"; + +export default class extends Controller { + @tracked saved = false; + @tracked selectedSiderbarCategories = []; + @tracked selectedSidebarTagNames = []; + + @action + tagUpdated(tagNames) { + this.selectedSidebarTagNames = tagNames; + this.model.set("sidebar_tag_names", tagNames); + this.saved = false; + } + + @action + categoryUpdated(categories) { + this.selectedSiderbarCategories = categories; + this.model.set("sidebarCategoryIds", categories.mapBy("id")); + this.saved = false; + } + + @action + save() { + this.model + .save() + .then(() => { + this.saved = true; + this.initialSidebarCategoryIds = this.model.sidebarCategoryIds; + this.initialSidebarTagNames = this.model.initialSidebarTagNames; + }) + .catch((error) => { + this.model.set("sidebarCategoryIds", this.initialSidebarCategoryIds); + this.model.set("sidebar_tag_names", this.initialSidebarTagNames); + popupAjaxError(error); + }); + } +} diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js index 086dbf4d379..b24c8d96365 100644 --- a/app/assets/javascripts/discourse/app/models/user.js +++ b/app/assets/javascripts/discourse/app/models/user.js @@ -1,7 +1,7 @@ import EmberObject, { computed, get, getProperties } from "@ember/object"; import cookie, { removeCookie } from "discourse/lib/cookie"; import { defaultHomepage, escapeExpression } from "discourse/lib/utilities"; -import { equal, filterBy, gt, or } from "@ember/object/computed"; +import { alias, equal, filterBy, gt, or } from "@ember/object/computed"; import getURL, { getURLWithCDN } from "discourse-common/lib/get-url"; import { A } from "@ember/array"; import Badge from "discourse/models/badge"; @@ -62,6 +62,8 @@ let userFields = [ "primary_group_id", "flair_group_id", "user_notification_schedule", + "sidebar_category_ids", + "sidebar_tag_names", ]; export function addSaveableUserField(fieldName) { @@ -307,6 +309,35 @@ const User = RestModel.extend({ @discourseComputed("silenced_till") silencedTillDate: longDate, + sidebarCategoryIds: alias("sidebar_category_ids"), + + @discourseComputed("sidebar_tag_names.[]") + sidebarTagNames(sidebarTagNames) { + if (!sidebarTagNames || sidebarTagNames.length === 0) { + return []; + } + + return sidebarTagNames; + }, + + @discourseComputed("sidebar_category_ids.[]") + sidebarCategories(sidebarCategoryIds) { + if (!sidebarCategoryIds || sidebarCategoryIds.length === 0) { + return []; + } + + return Site.current().categoriesList.filter((category) => { + if ( + this.siteSettings.suppress_uncategorized_badge && + category.isUncategorizedCategory + ) { + return false; + } + + return sidebarCategoryIds.includes(category.id); + }); + }, + changeUsername(new_username) { return ajax(userPath(`${this.username_lower}/preferences/username`), { type: "PUT", @@ -385,6 +416,12 @@ const User = RestModel.extend({ } }); + ["sidebar_category_ids", "sidebar_tag_names"].forEach((prop) => { + if (data[prop]?.length === 0) { + data[prop] = null; + } + }); + return this._saveUserData(data, updatedState); }, diff --git a/app/assets/javascripts/discourse/app/routes/app-route-map.js b/app/assets/javascripts/discourse/app/routes/app-route-map.js index e26451223ff..9306fa71dea 100644 --- a/app/assets/javascripts/discourse/app/routes/app-route-map.js +++ b/app/assets/javascripts/discourse/app/routes/app-route-map.js @@ -166,6 +166,7 @@ export default function () { this.route("tags"); this.route("interface"); this.route("apps"); + this.route("sidebar"); this.route("username"); this.route("email"); diff --git a/app/assets/javascripts/discourse/app/routes/preferences-sidebar.js b/app/assets/javascripts/discourse/app/routes/preferences-sidebar.js new file mode 100644 index 00000000000..f9d430144e9 --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/preferences-sidebar.js @@ -0,0 +1,20 @@ +import RestrictedUserRoute from "discourse/routes/restricted-user"; + +export default RestrictedUserRoute.extend({ + showFooter: true, + + setupController(controller, user) { + const props = { + model: user, + selectedSiderbarCategories: user.sidebarCategories, + initialSidebarCategoryIds: user.sidebarCategoryIds, + }; + + if (this.siteSettings.tagging_enabled) { + props.selectedSidebarTagNames = user.sidebarTagNames; + props.initialSidebarTagNames = user.sidebarTagNames; + } + + controller.setProperties(props); + }, +}); diff --git a/app/assets/javascripts/discourse/app/templates/components/sidebar/categories-section.hbs b/app/assets/javascripts/discourse/app/templates/components/sidebar/categories-section.hbs index 2fc5ebf8438..cd975451613 100644 --- a/app/assets/javascripts/discourse/app/templates/components/sidebar/categories-section.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/sidebar/categories-section.hbs @@ -2,7 +2,10 @@ @sectionName="categories" @headerRoute="discovery.categories" @headerLinkText={{i18n "sidebar.sections.categories.header_link_text"}} - @headerLinkTitle={{i18n "sidebar.sections.categories.header_link_title"}} > + @headerLinkTitle={{i18n "sidebar.sections.categories.header_link_title"}} + @headerAction={{this.editTracked}} + @headerActionTitle={{i18n "sidebar.sections.categories.header_action_title"}} + @headerActionIcon="pencil-alt" > {{#if (gt this.sectionLinks.length 0)}} {{#each this.sectionLinks as |sectionLink|}} @@ -19,7 +22,7 @@ {{/each}} {{else}} - {{i18n "sidebar.sections.categories.no_tracked_categories"}} + {{html-safe this.noCategoriesText}} {{/if}} diff --git a/app/assets/javascripts/discourse/app/templates/components/sidebar/tags-section.hbs b/app/assets/javascripts/discourse/app/templates/components/sidebar/tags-section.hbs index 78194b4de41..7e37080e946 100644 --- a/app/assets/javascripts/discourse/app/templates/components/sidebar/tags-section.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/sidebar/tags-section.hbs @@ -2,7 +2,10 @@ @sectionName="tags" @headerRoute="tags" @headerLinkText={{i18n "sidebar.sections.tags.header_link_text"}} - @headerLinkTitle={{i18n "sidebar.sections.tags.header_link_title"}} > + @headerLinkTitle={{i18n "sidebar.sections.tags.header_link_title"}} + @headerAction={{this.editTracked}} + @headerActionTitle={{i18n "sidebar.sections.tags.header_action_title"}} + @headerActionIcon="pencil-alt" > {{#if (gt this.sectionLinks.length 0)}} {{#each this.sectionLinks as |sectionLink|}} @@ -18,7 +21,7 @@ {{/each}} {{else}} - {{i18n "sidebar.sections.tags.no_tracked_tags"}} + {{html-safe this.noTagsText}} {{/if}} diff --git a/app/assets/javascripts/discourse/app/templates/preferences.hbs b/app/assets/javascripts/discourse/app/templates/preferences.hbs index f8b75a8522b..0d4c6446578 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences.hbs @@ -49,6 +49,13 @@ {{i18n "user.preferences_nav.interface"}} {{/link-to}} + {{#if this.currentUser.experimental_sidebar_enabled}} +
  • + {{#link-to "preferences.sidebar"}} + {{i18n "user.preferences_nav.sidebar"}} + {{/link-to}} +
    • + {{/if}} {{plugin-outlet name="user-preferences-nav-under-interface" tagName="span" connectorTagName="div" args=(hash model=model)}} diff --git a/app/assets/javascripts/discourse/app/templates/preferences/interface.hbs b/app/assets/javascripts/discourse/app/templates/preferences/interface.hbs index 77a8fbc517b..bbf00ca9008 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/interface.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences/interface.hbs @@ -133,10 +133,6 @@
    {{i18n "user.other_settings"}} - {{#if siteSettings.enable_experimental_sidebar}} - {{preference-checkbox labelKey="user.enable_experimental_sidebar" checked=model.user_option.enable_experimental_sidebar class="pref-external-links"}} - {{/if}} - {{preference-checkbox labelKey="user.external_links_in_new_tab" checked=model.user_option.external_links_in_new_tab class="pref-external-links"}} {{preference-checkbox labelKey="user.enable_quoting" checked=model.user_option.enable_quoting class="pref-enable-quoting"}} {{preference-checkbox labelKey="user.enable_defer" checked=model.user_option.enable_defer class="pref-defer-undread"}} diff --git a/app/assets/javascripts/discourse/app/templates/preferences/sidebar.hbs b/app/assets/javascripts/discourse/app/templates/preferences/sidebar.hbs new file mode 100644 index 00000000000..4489aae3064 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/preferences/sidebar.hbs @@ -0,0 +1,49 @@ +
    + {{i18n "user.experimental_sidebar.options"}} + + {{preference-checkbox + labelKey="user.experimental_sidebar.enable" + checked=model.user_option.enable_experimental_sidebar + class="preferences-sidebar-enable-checkbox"}} +
    + +{{#if model.experimental_sidebar_enabled}} +
    + {{i18n "user.experimental_sidebar.categories_section"}} + +
    + {{category-selector + categories=this.selectedSiderbarCategories + onChange=(action this.categoryUpdated) + options=(hash + allowUncategorized=(not this.siteSettings.suppress_uncategorized_badge) + displayCategoryDescription=true + ) + }} +
    + +
    {{i18n "user.experimental_sidebar.categories_section_instruction"}}
    +
    + + {{#if this.siteSettings.tagging_enabled}} +
    + {{i18n "user.experimental_sidebar.tags_section"}} + +
    + {{tag-chooser + tags=this.selectedSidebarTagNames + everyTag=true + unlimitedTagCount=true + onChange=(action this.tagUpdated) + options=(hash + allowAny=false + ) + }} +
    + +
    {{i18n "user.experimental_sidebar.tags_section_instruction"}}
    +
    + {{/if}} +{{/if}} + +{{save-controls model=model action=(action "save") saved=saved}} diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js index 9269776a2e2..01ddaa6d901 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js @@ -9,10 +9,9 @@ import { publishToMessageBus, query, queryAll, + updateCurrentUser, } from "discourse/tests/helpers/qunit-helpers"; -import selectKit from "discourse/tests/helpers/select-kit-helper"; import Site from "discourse/models/site"; -import { NotificationLevels } from "discourse/lib/notification-levels"; import discoveryFixture from "discourse/tests/fixtures/discovery-fixtures"; import categoryFixture from "discourse/tests/fixtures/category-fixtures"; import { cloneJSON } from "discourse-common/lib/object"; @@ -34,12 +33,9 @@ acceptance( return category.id === Site.current().uncategorized_category_id; }); - category1.set("notification_level", NotificationLevels.TRACKING); - - uncategorizedCategory.set( - "notification_level", - NotificationLevels.TRACKING - ); + updateCurrentUser({ + sidebar_category_ids: [category1.id, uncategorizedCategory.id], + }); await visit("/"); @@ -58,7 +54,11 @@ acceptance( ); acceptance("Sidebar - Categories Section", function (needs) { - needs.user({ experimental_sidebar_enabled: true }); + needs.user({ + experimental_sidebar_enabled: true, + sidebar_category_ids: [], + sidebar_tag_names: [], + }); needs.settings({ suppress_uncategorized_badge: false, @@ -76,19 +76,13 @@ acceptance("Sidebar - Categories Section", function (needs) { server.get("/c/:categorySlug/:categoryId/find_by_slug.json", () => { return helper.response(cloneJSON(categoryFixture["/c/1/show.json"])); }); - - server.post("/category/:categoryId/notifications", () => { - return helper.response({}); - }); }); - const setupTrackedCategories = function () { + const setupUserSidebarCategories = function () { const categories = Site.current().categories; const category1 = categories[0]; const category2 = categories[1]; - category1.set("notification_level", NotificationLevels.TRACKING); - category2.set("notification_level", NotificationLevels.TRACKING); - + updateCurrentUser({ sidebar_category_ids: [category1.id, category2.id] }); return { category1, category2 }; }; @@ -103,27 +97,42 @@ acceptance("Sidebar - Categories Section", function (needs) { ); }); - test("category section links when user does not have any tracked categories", async function (assert) { + test("clicking on section header button", async function (assert) { await visit("/"); + await click(".sidebar-section-categories .sidebar-section-header-button"); assert.strictEqual( - query(".sidebar-section-message").textContent.trim(), - I18n.t("sidebar.sections.categories.no_tracked_categories"), - "the no tracked categories message is displayed" + currentURL(), + "/u/eviltrout/preferences/sidebar", + "it should transition to user preferences sidebar page" ); }); - test("uncategorized category is shown when tracked", async function (assert) { + test("category section links when user has not added any categories", async function (assert) { + await visit("/"); + + assert.ok( + exists(".sidebar-section-message"), + "the no categories message is displayed" + ); + + await click(".sidebar-section-message a"); + + assert.strictEqual( + currentURL(), + "/u/eviltrout/preferences/sidebar", + "it should transition to user preferences sidebar page" + ); + }); + + test("uncategorized category is shown when added to sidebar", async function (assert) { const categories = Site.current().categories; const uncategorizedCategory = categories.find((category) => { - return category.id === Site.current().uncategorized_category_id; + return category.isUncategorizedCategory; }); - uncategorizedCategory.set( - "notification_level", - NotificationLevels.TRACKING - ); + updateCurrentUser({ sidebar_category_ids: [uncategorizedCategory.id] }); await visit("/"); @@ -133,8 +142,8 @@ acceptance("Sidebar - Categories Section", function (needs) { ); }); - test("category section links for tracked categories", async function (assert) { - const { category1, category2 } = setupTrackedCategories(); + test("category section links", async function (assert) { + const { category1, category2 } = setupUserSidebarCategories(); await visit("/"); @@ -196,8 +205,8 @@ acceptance("Sidebar - Categories Section", function (needs) { ); }); - test("visiting category discovery new route for tracked categories", async function (assert) { - const { category1 } = setupTrackedCategories(); + test("visiting category discovery new route", async function (assert) { + const { category1 } = setupUserSidebarCategories(); await visit(`/c/${category1.slug}/${category1.id}/l/new`); @@ -214,8 +223,8 @@ acceptance("Sidebar - Categories Section", function (needs) { ); }); - test("visiting category discovery unread route for tracked categories", async function (assert) { - const { category1 } = setupTrackedCategories(); + test("visiting category discovery unread route", async function (assert) { + const { category1 } = setupUserSidebarCategories(); await visit(`/c/${category1.slug}/${category1.id}/l/unread`); @@ -232,8 +241,8 @@ acceptance("Sidebar - Categories Section", function (needs) { ); }); - test("visiting category discovery top route for tracked categories", async function (assert) { - const { category1 } = setupTrackedCategories(); + test("visiting category discovery top route", async function (assert) { + const { category1 } = setupUserSidebarCategories(); await visit(`/c/${category1.slug}/${category1.id}/l/top`); @@ -250,58 +259,8 @@ acceptance("Sidebar - Categories Section", function (needs) { ); }); - test("updating category notification level", async function (assert) { - const { category1, category2 } = setupTrackedCategories(); - - await visit(`/c/${category1.slug}/${category1.id}/l/top`); - - assert.ok( - exists(`.sidebar-section-link-${category1.slug}`), - `has ${category1.name} section link in sidebar` - ); - - assert.ok( - exists(`.sidebar-section-link-${category2.slug}`), - `has ${category2.name} section link in sidebar` - ); - - const notificationLevelsDropdown = selectKit(".notifications-button"); - - await notificationLevelsDropdown.expand(); - - await notificationLevelsDropdown.selectRowByValue( - NotificationLevels.REGULAR - ); - - assert.ok( - !exists(`.sidebar-section-link-${category1.slug}`), - `does not have ${category1.name} section link in sidebar` - ); - - assert.ok( - exists(`.sidebar-section-link-${category2.slug}`), - `has ${category2.name} section link in sidebar` - ); - - await notificationLevelsDropdown.expand(); - - await notificationLevelsDropdown.selectRowByValue( - NotificationLevels.TRACKING - ); - - assert.ok( - exists(`.sidebar-section-link-${category1.slug}`), - `has ${category1.name} section link in sidebar` - ); - - assert.ok( - exists(`.sidebar-section-link-${category2.slug}`), - `has ${category2.name} section link in sidebar` - ); - }); - test("new and unread count for categories link", async function (assert) { - const { category1, category2 } = setupTrackedCategories(); + const { category1, category2 } = setupUserSidebarCategories(); this.container.lookup("topic-tracking-state:main").loadStates([ { @@ -426,7 +385,7 @@ acceptance("Sidebar - Categories Section", function (needs) { }); test("clean up topic tracking state state changed callbacks when section is destroyed", async function (assert) { - setupTrackedCategories(); + setupUserSidebarCategories(); await visit("/"); diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js index 17aa7a78612..4114da8b9c9 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js @@ -13,8 +13,6 @@ import { } from "discourse/tests/helpers/qunit-helpers"; import discoveryFixture from "discourse/tests/fixtures/discovery-fixtures"; import { cloneJSON } from "discourse-common/lib/object"; -import selectKit from "discourse/tests/helpers/select-kit-helper"; -import { NotificationLevels } from "discourse/lib/notification-levels"; acceptance("Sidebar - Tags section - tagging disabled", function (needs) { needs.settings({ @@ -43,6 +41,7 @@ acceptance("Sidebar - Tags section", function (needs) { tracked_tags: ["tag1"], watched_tags: ["tag2", "tag3"], watching_first_post_tags: [], + sidebar_tag_names: ["tag1", "tag2", "tag3"], }); needs.pretender((server, helper) => { @@ -59,16 +58,6 @@ acceptance("Sidebar - Tags section", function (needs) { ); }); }); - - server.put("/tag/:tagId/notifications", (request) => { - return helper.response({ - watched_tags: [], - watching_first_post_tags: [], - regular_tags: [request.params.tagId], - tracked_tags: [], - muted_tags: [], - }); - }); }); test("clicking on section header link", async function (assert) { @@ -82,11 +71,20 @@ acceptance("Sidebar - Tags section", function (needs) { ); }); - test("section content when user does not have any tracked tags", async function (assert) { + test("clicking on section header button", async function (assert) { + await visit("/"); + await click(".sidebar-section-tags .sidebar-section-header-button"); + + assert.strictEqual( + currentURL(), + "/u/eviltrout/preferences/sidebar", + "it should transition to user preferences sidebar page" + ); + }); + + test("section content when user has not added any tags", async function (assert) { updateCurrentUser({ - tracked_tags: [], - watched_tags: [], - watching_first_post_tags: [], + sidebar_tag_names: [], }); await visit("/"); @@ -95,12 +93,14 @@ acceptance("Sidebar - Tags section", function (needs) { query( ".sidebar-section-tags .sidebar-section-message" ).textContent.trim(), - I18n.t("sidebar.sections.tags.no_tracked_tags"), - "the no tracked tags message is displayed" + `${I18n.t("sidebar.sections.tags.none")} ${I18n.t( + "sidebar.sections.tags.click_to_get_started" + )}`, + "the no tags message is displayed" ); }); - test("tag section links for tracked tags", async function (assert) { + test("tag section links for user", async function (assert) { await visit("/"); assert.strictEqual( @@ -166,7 +166,7 @@ acceptance("Sidebar - Tags section", function (needs) { ); }); - test("visiting tag discovery top route for tracked tags", async function (assert) { + test("visiting tag discovery top route", async function (assert) { await visit(`/tag/tag1/l/top`); assert.strictEqual( @@ -181,7 +181,7 @@ acceptance("Sidebar - Tags section", function (needs) { ); }); - test("visiting tag discovery new route for tracked tags", async function (assert) { + test("visiting tag discovery new ", async function (assert) { await visit(`/tag/tag1/l/new`); assert.strictEqual( @@ -196,7 +196,7 @@ acceptance("Sidebar - Tags section", function (needs) { ); }); - test("visiting tag discovery unread route for tracked tags", async function (assert) { + test("visiting tag discovery unread route", async function (assert) { await visit(`/tag/tag1/l/unread`); assert.strictEqual( @@ -341,21 +341,4 @@ acceptance("Sidebar - Tags section", function (needs) { initialCallbackCount ); }); - - test("updating tags notification levels", async function (assert) { - await visit(`/tag/tag1/l/unread`); - - const notificationLevelsDropdown = selectKit(".notifications-button"); - - await notificationLevelsDropdown.expand(); - - await notificationLevelsDropdown.selectRowByValue( - NotificationLevels.REGULAR - ); - - assert.ok( - !exists(".sidebar-section-tags .sidebar-section-link-tag1"), - "tag1 section link is removed from sidebar" - ); - }); }); diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js new file mode 100644 index 00000000000..6e486d53d29 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js @@ -0,0 +1,213 @@ +import { test } from "qunit"; + +import { click, visit } from "@ember/test-helpers"; + +import { + acceptance, + exists, + updateCurrentUser, +} from "discourse/tests/helpers/qunit-helpers"; +import selectKit from "discourse/tests/helpers/select-kit-helper"; + +acceptance("User Preferences - Sidebar - Tagging Disabled", function (needs) { + needs.settings({ + tagging_enabled: false, + }); + + needs.user({ + experimental_sidebar_enabled: true, + sidebar_category_ids: [], + }); + + test("user should not see tag chooser", async function (assert) { + await visit("/u/eviltrout/preferences/sidebar"); + + assert.ok(!exists(".tag-chooser"), "tag chooser is not displayed"); + }); +}); + +acceptance("User Preferences - Sidebar", function (needs) { + needs.user({ + experimental_sidebar_enabled: true, + sidebar_category_ids: [], + sidebar_tag_names: [], + }); + + needs.settings({ + tagging_enabled: true, + }); + + let updateUserRequestBody = null; + + needs.hooks.afterEach(() => { + updateUserRequestBody = null; + }); + + needs.pretender((server, helper) => { + server.put("/u/eviltrout.json", (request) => { + updateUserRequestBody = helper.parsePostData(request.requestBody); + + // if only the howto category is updated, intentionally cause an error + if ( + updateUserRequestBody["sidebar_category_ids[]"]?.[0] === "10" || + updateUserRequestBody["sidebar_tag_names[]"]?.[0] === "gazelle" + ) { + // This request format will cause an error + return helper.response(400, {}); + } else { + return helper.response({ user: {} }); + } + }); + }); + + test("user encountering error when adding categories to sidebar", async function (assert) { + updateCurrentUser({ sidebar_category_ids: [6] }); + + await visit("/"); + + assert.ok( + exists(".sidebar-section-categories .sidebar-section-link-support"), + "support category is present in sidebar" + ); + + await click(".sidebar-section-categories .sidebar-section-header-button"); + + const categorySelector = selectKit(".category-selector"); + await categorySelector.expand(); + await categorySelector.selectKitSelectRowByName("howto"); + await categorySelector.deselectItemByName("support"); + + assert.ok( + exists(".sidebar-section-categories .sidebar-section-link-howto"), + "howto category has been added to sidebar" + ); + + await click(".save-changes"); + + assert.deepEqual( + updateUserRequestBody["sidebar_category_ids[]"], + ["10"], + "contains the right request body to update user's sidebar category links" + ); + + assert.ok(exists(".modal-body"), "error message is displayed"); + + await click(".modal .d-button-label"); + + assert.ok( + !exists(".sidebar-section-categories .sidebar-section-link-howto"), + "howto category has been removed from sidebar" + ); + + assert.ok( + exists(".sidebar-section-categories .sidebar-section-link-support"), + "support category is added back to sidebar" + ); + }); + + test("user adding categories to sidebar", async function (assert) { + await visit("/"); + await click(".sidebar-section-categories .sidebar-section-header-button"); + + const categorySelector = selectKit(".category-selector"); + await categorySelector.expand(); + await categorySelector.selectKitSelectRowByName("support"); + await categorySelector.selectKitSelectRowByName("bug"); + + assert.ok( + exists(".sidebar-section-categories .sidebar-section-link-support"), + "support category has been added to sidebar" + ); + + assert.ok( + exists(".sidebar-section-categories .sidebar-section-link-bug"), + "bug category has been added to sidebar" + ); + + await click(".save-changes"); + + assert.deepEqual( + updateUserRequestBody["sidebar_category_ids[]"], + ["6", "1"], + "contains the right request body to update user's sidebar category links" + ); + }); + + test("user encountering error when adding tags to sidebar", async function (assert) { + updateCurrentUser({ sidebar_tag_names: ["monkey"] }); + + await visit("/"); + + assert.ok( + exists(".sidebar-section-tags .sidebar-section-link-monkey"), + "monkey tag is present in sidebar" + ); + + await click(".sidebar-section-tags .sidebar-section-header-button"); + + const tagChooser = selectKit(".tag-chooser"); + await tagChooser.expand(); + await tagChooser.selectKitSelectRowByName("gazelle"); + await tagChooser.deselectItemByName("monkey"); + + assert.ok( + exists(".sidebar-section-tags .sidebar-section-link-gazelle"), + "gazelle tag has been added to sidebar" + ); + + assert.ok( + !exists(".sidebar-section-tags .sidebar-section-link-monkey"), + "monkey tag has been removed from sidebar" + ); + + await click(".save-changes"); + + assert.deepEqual( + updateUserRequestBody["sidebar_tag_names[]"], + ["gazelle"], + "contains the right request body to update user's sidebar tag links" + ); + + assert.ok(exists(".modal-body"), "error message is displayed"); + + await click(".modal .d-button-label"); + + assert.ok( + !exists(".sidebar-section-tags .sidebar-section-link-gazelle"), + "gazelle tag has been removed from sidebar" + ); + + assert.ok( + exists(".sidebar-section-tags .sidebar-section-link-monkey"), + "monkey tag is added back to sidebar" + ); + }); + + test("user adding tags to sidebar", async function (assert) { + await visit("/"); + await click(".sidebar-section-tags .sidebar-section-header-button"); + + const tagChooser = selectKit(".tag-chooser"); + await tagChooser.expand(); + await tagChooser.selectKitSelectRowByName("monkey"); + await tagChooser.selectKitSelectRowByName("gazelle"); + + assert.ok( + exists(".sidebar-section-tags .sidebar-section-link-monkey"), + "monkey tag has been added to sidebar" + ); + + assert.ok( + exists(".sidebar-section-tags .sidebar-section-link-gazelle"), + "gazelle tag has been added to sidebar" + ); + + await click(".save-changes"); + + assert.deepEqual( + updateUserRequestBody["sidebar_tag_names[]"], + ["monkey", "gazelle"], + "contains the right request body to update user's sidebar tag links" + ); + }); +}); diff --git a/app/assets/javascripts/discourse/tests/helpers/create-pretender.js b/app/assets/javascripts/discourse/tests/helpers/create-pretender.js index dbabe18be01..149845fd4f4 100644 --- a/app/assets/javascripts/discourse/tests/helpers/create-pretender.js +++ b/app/assets/javascripts/discourse/tests/helpers/create-pretender.js @@ -10,12 +10,16 @@ export function parsePostData(query) { const item = part.split("="); const firstSeg = decodeURIComponent(item[0]); const m = /^([^\[]+)\[(.+)\]/.exec(firstSeg); - const val = decodeURIComponent(item[1]).replace(/\+/g, " "); + const isArray = firstSeg.endsWith("[]"); + if (m) { let key = m[1]; result[key] = result[key] || {}; result[key][m[2].replace("][", ".")] = val; + } else if (isArray) { + result[firstSeg] ||= []; + result[firstSeg].push(val); } else { result[firstSeg] = val; } diff --git a/app/assets/javascripts/discourse/tests/helpers/select-kit-helper.js b/app/assets/javascripts/discourse/tests/helpers/select-kit-helper.js index 4fb02bee758..aaa15afe3ce 100644 --- a/app/assets/javascripts/discourse/tests/helpers/select-kit-helper.js +++ b/app/assets/javascripts/discourse/tests/helpers/select-kit-helper.js @@ -298,6 +298,10 @@ export default function selectKit(selector) { await click(`${selector} .selected-content [data-value="${value}"]`); }, + async deselectItemByName(name) { + await click(`${selector} .selected-content [data-name="${name}"]`); + }, + exists() { return exists(selector); }, diff --git a/app/assets/javascripts/select-kit/addon/components/category-selector.js b/app/assets/javascripts/select-kit/addon/components/category-selector.js index 948d4127817..b5a2cd036d8 100644 --- a/app/assets/javascripts/select-kit/addon/components/category-selector.js +++ b/app/assets/javascripts/select-kit/addon/components/category-selector.js @@ -16,7 +16,7 @@ export default MultiSelectComponent.extend({ selectKitOptions: { filterable: true, allowAny: false, - allowUncategorized: "allowUncategorized", + allowUncategorized: true, displayCategoryDescription: false, selectedChoiceComponent: "selected-choice-category", }, @@ -35,6 +35,14 @@ export default MultiSelectComponent.extend({ content: computed("categories.[]", "blockedCategories.[]", function () { const blockedCategories = makeArray(this.blockedCategories); return Category.list().filter((category) => { + if (category.isUncategorizedCategory) { + if (this.attrs.options?.allowUncategorized !== undefined) { + return this.attrs.options.allowUncategorized; + } + + return this.selectKitOptions.allowUncategorized; + } + return ( this.categories.includes(category) || !blockedCategories.includes(category) diff --git a/app/assets/stylesheets/common/select-kit/category-row.scss b/app/assets/stylesheets/common/select-kit/category-row.scss index ed7c57eba1b..4da20524c10 100644 --- a/app/assets/stylesheets/common/select-kit/category-row.scss +++ b/app/assets/stylesheets/common/select-kit/category-row.scss @@ -1,5 +1,8 @@ .select-kit { - .category-row { + .select-kit-row.category-row { + flex-direction: column; + align-items: flex-start; + .category-status { display: flex; align-items: center; @@ -7,6 +10,10 @@ -webkit-box-flex: 0; -ms-flex: 1 1 auto; flex: 1 1 auto; + + .category-name { + color: var(--primary); + } } .category-desc p { margin: 0; @@ -17,6 +24,11 @@ margin-top: 1px; } } + .category-desc { + margin: 0; + font-size: var(--font-down-1); + color: var(--primary-medium); + } .topic-count { white-space: nowrap; } diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index e552bc67e2e..c3e22064521 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1814,7 +1814,7 @@ class UsersController < ApplicationController :card_background_upload_url, :primary_group_id, :flair_group_id, - :featured_topic_id + :featured_topic_id, ] editable_custom_fields = User.editable_user_custom_fields(by_staff: current_user.try(:staff?)) @@ -1824,6 +1824,22 @@ class UsersController < ApplicationController permitted.concat UserUpdater::TAG_NAMES.keys permitted << UserUpdater::NOTIFICATION_SCHEDULE_ATTRS + if current_user&.user_option&.enable_experimental_sidebar + if params.has_key?(:sidebar_category_ids) && params[:sidebar_category_ids].blank? + params[:sidebar_category_ids] = [] + end + + permitted << { sidebar_category_ids: [] } + + if SiteSetting.tagging_enabled + if params.has_key?(:sidebar_tag_names) && params[:sidebar_tag_names].blank? + params[:sidebar_tag_names] = [] + end + + permitted << { sidebar_tag_names: [] } + end + end + result = params .permit(permitted, theme_ids: []) .reverse_merge( diff --git a/app/models/category.rb b/app/models/category.rb index 9b1201bfc4d..f9052ce3979 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -113,6 +113,7 @@ class Category < ActiveRecord::Base has_many :tag_groups, through: :category_tag_groups has_many :category_required_tag_groups, -> { order(order: :asc) }, dependent: :destroy + has_many :sidebar_section_links, as: :linkable, dependent: :delete_all belongs_to :reviewable_by_group, class_name: 'Group' diff --git a/app/models/sidebar_section_link.rb b/app/models/sidebar_section_link.rb new file mode 100644 index 00000000000..0b6f7bfebea --- /dev/null +++ b/app/models/sidebar_section_link.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class SidebarSectionLink < ActiveRecord::Base + belongs_to :user + belongs_to :linkable, polymorphic: true + + validates :user_id, presence: true, uniqueness: { scope: [:linkable_type, :linkable_id] } + validates :linkable_id, presence: true + validates :linkable_type, presence: true + validate :ensure_supported_linkable_type, if: :will_save_change_to_linkable_type? + + SUPPORTED_LINKABLE_TYPES = %w{Category Tag} + + private def ensure_supported_linkable_type + if (!SUPPORTED_LINKABLE_TYPES.include?(self.linkable_type)) || (self.linkable_type == 'Tag' && !SiteSetting.tagging_enabled) + self.errors.add(:linkable_type, I18n.t("activerecord.errors.models.sidebar_section_link.attributes.linkable_type.invalid")) + end + end +end + +# == Schema Information +# +# Table name: sidebar_section_links +# +# id :bigint not null, primary key +# user_id :integer not null +# linkable_id :integer not null +# linkable_type :string not null +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# idx_unique_sidebar_section_links (user_id,linkable_type,linkable_id) UNIQUE +# diff --git a/app/models/tag.rb b/app/models/tag.rb index 7a98de7cdcb..3d5224d4b98 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -45,6 +45,7 @@ class Tag < ActiveRecord::Base belongs_to :target_tag, class_name: "Tag", optional: true has_many :synonyms, class_name: "Tag", foreign_key: "target_tag_id", dependent: :destroy + has_many :sidebar_section_links, as: :linkable, dependent: :delete_all after_save :index_search after_save :update_synonym_associations diff --git a/app/models/user.rb b/app/models/user.rb index b7c040c580c..defb186e0af 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -107,6 +107,10 @@ class User < ActiveRecord::Base belongs_to :uploaded_avatar, class_name: 'Upload' + has_many :sidebar_section_links, dependent: :delete_all + has_many :category_sidebar_section_links, -> { where(linkable_type: "Category") }, class_name: 'SidebarSectionLink' + has_many :sidebar_tags, through: :sidebar_section_links, source: :linkable, source_type: "Tag" + delegate :last_sent_email_address, to: :email_logs validates_presence_of :username diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index 23b59e70cfb..6ead0ceb5da 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -72,7 +72,9 @@ class CurrentUserSerializer < BasicUserSerializer :bookmark_auto_delete_preference, :pending_posts_count, :experimental_sidebar_enabled, - :status + :status, + :sidebar_category_ids, + :sidebar_tag_names delegate :user_stat, to: :object, private: true delegate :any_posts, :draft_count, :pending_posts_count, :read_faq?, to: :user_stat @@ -313,6 +315,22 @@ class CurrentUserSerializer < BasicUserSerializer SiteSetting.enable_experimental_sidebar end + def sidebar_category_ids + object.category_sidebar_section_links.pluck(:linkable_id) + end + + def include_sidebar_category_ids? + include_experimental_sidebar_enabled? && object.user_option.enable_experimental_sidebar + end + + def sidebar_tag_names + object.sidebar_tags.pluck(:name) + end + + def include_sidebar_tag_names? + include_sidebar_category_ids? && SiteSetting.tagging_enabled + end + def include_status? SiteSetting.enable_user_status end diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index aaed136b5b9..5190bdbfc8c 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -202,6 +202,14 @@ class UserUpdater updated_associated_accounts(attributes[:user_associated_accounts]) end + if attributes.key?(:sidebar_category_ids) + update_sidebar_category_section_links(attributes[:sidebar_category_ids]) + end + + if attributes.key?(:sidebar_tag_names) && SiteSetting.tagging_enabled + update_sidebar_tag_section_links(attributes[:sidebar_tag_names]) + end + name_changed = user.name_changed? if (saved = (!save_options || user.user_option.save) && (user_notification_schedule.nil? || user_notification_schedule.save) && user_profile.save && user.save) && (name_changed && old_user_name.casecmp(attributes.fetch(:name)) != 0) @@ -283,6 +291,48 @@ class UserUpdater private + def delete_all_sidebar_section_links(linkable_type) + SidebarSectionLink.where(user: user, linkable_type: linkable_type).delete_all + end + + def update_sidebar_section_links(linkable_type, new_linkable_ids) + if new_linkable_ids.blank? + SidebarSectionLink.where(user: user, linkable_type: linkable_type).delete_all + else + existing_linkable_ids = SidebarSectionLink.where(user: user, linkable_type: linkable_type).pluck(:linkable_id) + + to_delete = existing_linkable_ids - new_linkable_ids + to_insert = new_linkable_ids - existing_linkable_ids + + to_insert_attributes = to_insert.map do |linkable_id| + { + linkable_type: linkable_type, + linkable_id: linkable_id, + user_id: user.id + } + end + + SidebarSectionLink.where(user: user, linkable_type: linkable_type, linkable_id: to_delete).delete_all if to_delete.present? + SidebarSectionLink.insert_all(to_insert_attributes) if to_insert_attributes.present? + end + end + + def update_sidebar_tag_section_links(tag_names) + if tag_names.blank? + delete_all_sidebar_section_links('Tag') + else + update_sidebar_section_links('Tag', Tag.where(name: tag_names).pluck(:id)) + end + end + + def update_sidebar_category_section_links(category_ids) + if category_ids.blank? + delete_all_sidebar_section_links('Category') + else + update_sidebar_section_links('Category', Category.secured(guardian).where(id: category_ids).pluck(:id)) + end + end + attr_reader :user, :guardian def format_url(website) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 17103dc056a..42dd489e4a6 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1161,7 +1161,13 @@ en: external_links_in_new_tab: "Open all external links in a new tab" enable_quoting: "Enable quote reply for highlighted text" enable_defer: "Enable defer to mark topics unread" - enable_experimental_sidebar: "Enable experimental sidebar" + experimental_sidebar: + enable: "Enable experimental sidebar" + options: "Options" + categories_section: "Categories Section" + categories_section_instruction: "Selected categories will be displayed under Sidebar's categories section." + tags_section: "Tags Section" + tags_section_instruction: "Selected tags will be displayed under Sidebar's tags section." change: "change" featured_topic: "Featured Topic" moderator: "%{user} is a moderator" @@ -1306,6 +1312,7 @@ en: tags: "Tags" interface: "Interface" apps: "Apps" + sidebar: "Sidebar" change_password: success: "(email sent)" @@ -4056,13 +4063,17 @@ en: unread_with_count: "Unread (%{count})" archive: "Archive" tags: - no_tracked_tags: "You are not tracking any tags." + none: "You have not added any tags." + click_to_get_started: "Click here to get started." header_link_title: "all tags" header_link_text: "Tags" + header_action_title: "edit your sidebar tags" categories: - no_tracked_categories: "You are not tracking any categories." + none: "You have not added any categories." + click_to_get_started: "Click here to get started." header_link_title: "all categories" header_link_text: "Categories" + header_action_title: "edit your sidebar categories" topics: header_link_title: "home" header_link_text: "Topics" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 3e40272aa7e..a64031674ae 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -640,6 +640,11 @@ en: base: invalid_url: "Replacement URL is invalid" invalid_tag_list: "Replacement tag list is invalid" + sidebar_section_link: + attributes: + linkable_type: + invalid: "is not valid" + uncategorized_category_name: "Uncategorized" diff --git a/config/routes.rb b/config/routes.rb index 3f8954ea9e9..2772b16e868 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -488,6 +488,7 @@ Discourse::Application.routes.draw do get "#{root_path}/:username/preferences/users" => "users#preferences", constraints: { username: RouteFormat.username } get "#{root_path}/:username/preferences/tags" => "users#preferences", constraints: { username: RouteFormat.username } get "#{root_path}/:username/preferences/interface" => "users#preferences", constraints: { username: RouteFormat.username } + get "#{root_path}/:username/preferences/sidebar" => "users#preferences", constraints: { username: RouteFormat.username } get "#{root_path}/:username/preferences/apps" => "users#preferences", constraints: { username: RouteFormat.username } post "#{root_path}/:username/preferences/email" => "users_email#create", constraints: { username: RouteFormat.username } put "#{root_path}/:username/preferences/email" => "users_email#update", constraints: { username: RouteFormat.username } diff --git a/db/migrate/20220628031850_create_sidebar_section_links.rb b/db/migrate/20220628031850_create_sidebar_section_links.rb new file mode 100644 index 00000000000..3bd1bad37fa --- /dev/null +++ b/db/migrate/20220628031850_create_sidebar_section_links.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CreateSidebarSectionLinks < ActiveRecord::Migration[7.0] + def change + create_table :sidebar_section_links do |t| + t.integer :user_id, null: false + t.integer :linkable_id, null: false + t.string :linkable_type, null: false + + t.timestamps + end + + add_index :sidebar_section_links, [:user_id, :linkable_type, :linkable_id], unique: true, name: 'idx_unique_sidebar_section_links' + end +end diff --git a/spec/fabricators/sidebar_section_link_fabricator.rb b/spec/fabricators/sidebar_section_link_fabricator.rb new file mode 100644 index 00000000000..4a5d7683807 --- /dev/null +++ b/spec/fabricators/sidebar_section_link_fabricator.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +Fabricator(:sidebar_section_link) do + user +end + +Fabricator(:category_sidebar_section_link, from: :sidebar_section_link) do + linkable(fabricator: :category) +end + +Fabricator(:tag_sidebar_section_link, from: :sidebar_section_link) do + linkable(fabricator: :tag) +end diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb index 8714c40ec4a..1eb38821284 100644 --- a/spec/models/category_spec.rb +++ b/spec/models/category_spec.rb @@ -30,6 +30,17 @@ describe Category do expect(cats.errors[:name]).to be_present end + context 'associations' do + it 'should delete associated sidebar_section_links when category is destroyed' do + category_sidebar_section_link = Fabricate(:category_sidebar_section_link) + category_sidebar_section_link_2 = Fabricate(:category_sidebar_section_link, linkable: category_sidebar_section_link.linkable) + tag_sidebar_section_link = Fabricate(:tag_sidebar_section_link) + + expect { category_sidebar_section_link.linkable.destroy! }.to change { SidebarSectionLink.count }.from(3).to(1) + expect(SidebarSectionLink.first).to eq(tag_sidebar_section_link) + end + end + describe "slug" do it "converts to lower" do category = Category.create!(name: "Hello World", slug: "Hello-World", user: user) diff --git a/spec/models/sidebar_section_link_spec.rb b/spec/models/sidebar_section_link_spec.rb new file mode 100644 index 00000000000..d4456dc2ab9 --- /dev/null +++ b/spec/models/sidebar_section_link_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +RSpec.describe SidebarSectionLink do + fab!(:user) { Fabricate(:user) } + + context "validations" do + it 'is not valid when linkable already exists for the current user' do + category_sidebar_section_link = Fabricate(:category_sidebar_section_link, user: user) + + sidebar_section_link = SidebarSectionLink.new(user: user, linkable: category_sidebar_section_link.linkable) + + expect(sidebar_section_link.valid?).to eq(false) + expect(sidebar_section_link.errors.details[:user_id][0][:error]).to eq(:taken) + end + + describe '#linkable_type' do + it "is not valid when linkable_type is not supported" do + sidebar_section_link = SidebarSectionLink.new(user: user, linkable_id: 1, linkable_type: 'sometype') + + expect(sidebar_section_link.valid?).to eq(false) + + expect(sidebar_section_link.errors[:linkable_type]).to eq([ + I18n.t("activerecord.errors.models.sidebar_section_link.attributes.linkable_type.invalid") + ]) + end + + it "is not valid when linkable_type is Tag and SiteSetting.tagging_enabled is false" do + SiteSetting.tagging_enabled = false + sidebar_section_link = SidebarSectionLink.new(user: user, linkable_id: 1, linkable_type: 'Tag') + + expect(sidebar_section_link.valid?).to eq(false) + + expect(sidebar_section_link.errors[:linkable_type]).to eq([ + I18n.t("activerecord.errors.models.sidebar_section_link.attributes.linkable_type.invalid") + ]) + end + end + end +end diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index 8f68f476aa0..ee4e354da64 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -13,12 +13,24 @@ describe Tag do let(:tag) { Fabricate(:tag) } let(:tag2) { Fabricate(:tag) } let(:topic) { Fabricate(:topic, tags: [tag]) } + fab!(:user) { Fabricate(:user) } before do SiteSetting.tagging_enabled = true SiteSetting.min_trust_level_to_tag_topics = 0 end + context 'associations' do + it 'should delete associated sidebar_section_links when tag is destroyed' do + tag_sidebar_section_link = Fabricate(:tag_sidebar_section_link) + tag_sidebar_section_link_2 = Fabricate(:tag_sidebar_section_link, linkable: tag_sidebar_section_link.linkable) + category_sidebar_section_link = Fabricate(:category_sidebar_section_link) + + expect { tag_sidebar_section_link.linkable.destroy! }.to change { SidebarSectionLink.count }.from(3).to(1) + expect(SidebarSectionLink.first).to eq(category_sidebar_section_link) + end + end + describe 'new' do subject { Fabricate.build(:tag) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 11596fa911e..27e4af6f78e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -11,6 +11,15 @@ RSpec.describe User do it { is_expected.to have_many(:pending_posts).class_name('ReviewableQueuedPost').with_foreign_key(:created_by_id) } + context 'associations' do + it 'should delete sidebar_section_links when a user is destroyed' do + Fabricate(:category_sidebar_section_link, user: user) + Fabricate(:tag_sidebar_section_link, user: user) + + expect { user.destroy! }.to change { SidebarSectionLink.where(user: user).count }.from(2).to(0) + end + end + context 'validations' do describe '#username' do it { is_expected.to validate_presence_of :username } diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 1b8342538fd..03e157ef899 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -2297,6 +2297,102 @@ describe UsersController do json = response.parsed_body expect(json['user']['id']).to eq user.id end + + context 'experimental sidebar' do + before do + SiteSetting.enable_experimental_sidebar = true + user.user_option.update!(enable_experimental_sidebar: true) + end + + it "should allow user to update UserOption#enable_experimental_sidebar" do + put "/u/#{user.username}.json", params: { enable_experimental_sidebar: 'false' } + + expect(response.status).to eq(200) + expect(user.reload.user_option.enable_experimental_sidebar).to eq(false) + + put "/u/#{user.username}.json", params: { enable_experimental_sidebar: 'true' } + + expect(response.status).to eq(200) + expect(user.reload.user_option.enable_experimental_sidebar).to eq(true) + end + + it 'does not remove category or tag sidebar section links when params are not present' do + Fabricate(:category_sidebar_section_link, user: user) + Fabricate(:tag_sidebar_section_link, user: user) + + expect do + put "/u/#{user.username}.json" + + expect(response.status).to eq(200) + end.to_not change { user.sidebar_section_links.count } + end + + it "should allow user to remove all category sidebar section links" do + Fabricate(:category_sidebar_section_link, user: user) + + expect do + put "/u/#{user.username}.json", params: { sidebar_category_ids: nil } + + expect(response.status).to eq(200) + end.to change { user.sidebar_section_links.count }.from(1).to(0) + end + + it "should allow user to modify category sidebar section links" do + category = Fabricate(:category) + restricted_category = Fabricate(:category, read_restricted: true) + category_siderbar_section_link = Fabricate(:category_sidebar_section_link, user: user) + + put "/u/#{user.username}.json", params: { sidebar_category_ids: [category.id, restricted_category.id] } + + expect(response.status).to eq(200) + expect(user.sidebar_section_links.count).to eq(1) + expect(SidebarSectionLink.exists?(id: category_siderbar_section_link.id)).to eq(false) + + sidebar_section_link = user.sidebar_section_links.first + + expect(sidebar_section_link.linkable).to eq(category) + end + + it 'should allow user to remove all tag sidebar section links' do + SiteSetting.tagging_enabled = true + + Fabricate(:tag_sidebar_section_link, user: user) + + expect do + put "/u/#{user.username}.json", params: { sidebar_tag_names: nil } + + expect(response.status).to eq(200) + end.to change { user.sidebar_section_links.count }.from(1).to(0) + end + + it 'should not allow user to add tag sidebar section links when tagging is disabled' do + SiteSetting.tagging_enabled = false + + tag = Fabricate(:tag) + + put "/u/#{user.username}.json", params: { sidebar_tag_names: [tag.name] } + + expect(response.status).to eq(200) + expect(user.reload.sidebar_section_links.count).to eq(0) + end + + it "should allow user to add tag sidebar section links" do + SiteSetting.tagging_enabled = true + + tag = Fabricate(:tag) + tag_sidebar_section_link = Fabricate(:tag_sidebar_section_link, user: user) + + put "/u/#{user.username}.json", params: { sidebar_tag_names: [tag.name, "somerandomtag"] } + + expect(response.status).to eq(200) + expect(user.sidebar_section_links.count).to eq(1) + expect(SidebarSectionLink.exists?(id: tag_sidebar_section_link.id)).to eq(false) + + sidebar_section_link = user.sidebar_section_links.first + + expect(sidebar_section_link.linkable).to eq(tag) + end + end end context 'without permission to update' do @@ -2309,21 +2405,6 @@ describe UsersController do expect(response).to be_forbidden expect(user.reload.name).not_to eq 'Jim Tom' end - - context 'enabling experimental sidebar' do - before do - sign_in(user) - end - - it "should be able to update UserOption#enable_experimental_sidebar" do - SiteSetting.enable_experimental_sidebar = true - - put "/u/#{user.username}.json", params: { enable_experimental_sidebar: 'true' } - - expect(response.status).to eq(200) - expect(user.user_option.enable_experimental_sidebar).to eq(true) - end - end end end diff --git a/spec/serializers/current_user_serializer_spec.rb b/spec/serializers/current_user_serializer_spec.rb index bf2ec182ba5..55eab6fc944 100644 --- a/spec/serializers/current_user_serializer_spec.rb +++ b/spec/serializers/current_user_serializer_spec.rb @@ -1,13 +1,12 @@ # frozen_string_literal: true RSpec.describe CurrentUserSerializer do + fab!(:user) { Fabricate(:user) } subject(:serializer) { described_class.new(user, scope: guardian, root: false) } - let(:guardian) { Guardian.new } + let(:guardian) { Guardian.new(user) } context "when SSO is not enabled" do - fab!(:user) { Fabricate(:user) } - it "should not include the external_id field" do payload = serializer.as_json expect(payload).not_to have_key(:external_id) @@ -31,7 +30,6 @@ RSpec.describe CurrentUserSerializer do end context "#top_category_ids" do - fab!(:user) { Fabricate(:user) } fab!(:category1) { Fabricate(:category) } fab!(:category2) { Fabricate(:category) } fab!(:category3) { Fabricate(:category) } @@ -61,7 +59,6 @@ RSpec.describe CurrentUserSerializer do end context "#muted_tag" do - fab!(:user) { Fabricate(:user) } fab!(:tag) { Fabricate(:tag) } let!(:tag_user) do @@ -79,7 +76,6 @@ RSpec.describe CurrentUserSerializer do end context "#second_factor_enabled" do - fab!(:user) { Fabricate(:user) } let(:guardian) { Guardian.new(user) } let(:json) { serializer.as_json } @@ -109,8 +105,6 @@ RSpec.describe CurrentUserSerializer do end context "#groups" do - fab!(:user) { Fabricate(:user) } - it "should only show visible groups" do Fabricate.build(:group, visibility_level: Group.visibility_levels[:public]) hidden_group = Fabricate.build(:group, visibility_level: Group.visibility_levels[:owners]) @@ -128,8 +122,6 @@ RSpec.describe CurrentUserSerializer do end context "#has_topic_draft" do - fab!(:user) { Fabricate(:user) } - it "is not included by default" do payload = serializer.as_json expect(payload).not_to have_key(:has_topic_draft) @@ -207,4 +199,83 @@ RSpec.describe CurrentUserSerializer do expect(json.keys).not_to include :status end end + + describe '#sidebar_tag_names' do + fab!(:tag_sidebar_section_link) { Fabricate(:tag_sidebar_section_link, user: user) } + fab!(:tag_sidebar_section_link_2) { Fabricate(:tag_sidebar_section_link, user: user) } + + it "is not included when SiteSeting.enable_experimental_sidebar is false" do + SiteSetting.enable_experimental_sidebar = false + + json = serializer.as_json + + expect(json[:sidebar_tag_names]).to eq(nil) + end + + it "is not included when SiteSeting.tagging_enabled is false" do + SiteSetting.enable_experimental_sidebar = true + SiteSetting.tagging_enabled = false + + json = serializer.as_json + + expect(json[:sidebar_tag_names]).to eq(nil) + end + + it "is not included when experimental sidebar has not been enabled by user" do + SiteSetting.enable_experimental_sidebar = true + SiteSetting.tagging_enabled = true + user.user_option.update!(enable_experimental_sidebar: false) + + json = serializer.as_json + + expect(json[:sidebar_tag_names]).to eq(nil) + end + + it "is present when experimental sidebar has been enabled by user" do + SiteSetting.enable_experimental_sidebar = true + SiteSetting.tagging_enabled = true + user.user_option.update!(enable_experimental_sidebar: true) + + json = serializer.as_json + + expect(json[:sidebar_tag_names]).to contain_exactly( + tag_sidebar_section_link.linkable.name, + tag_sidebar_section_link_2.linkable.name + ) + end + end + + describe '#sidebar_category_ids' do + fab!(:category_sidebar_section_link) { Fabricate(:category_sidebar_section_link, user: user) } + fab!(:category_sidebar_section_link_2) { Fabricate(:category_sidebar_section_link, user: user) } + + it "is not included when SiteSeting.enable_experimental_sidebar is false" do + SiteSetting.enable_experimental_sidebar = false + + json = serializer.as_json + + expect(json[:sidebar_category_ids]).to eq(nil) + end + + it "is not included when experimental sidebar has not been enabled by user" do + SiteSetting.enable_experimental_sidebar = true + user.user_option.update!(enable_experimental_sidebar: false) + + json = serializer.as_json + + expect(json[:sidebar_category_ids]).to eq(nil) + end + + it "is present when experimental sidebar has been enabled by user" do + SiteSetting.enable_experimental_sidebar = true + user.user_option.update!(enable_experimental_sidebar: true) + + json = serializer.as_json + + expect(json[:sidebar_category_ids]).to contain_exactly( + category_sidebar_section_link.linkable_id, + category_sidebar_section_link_2.linkable_id + ) + end + end end