From 5cc78c7b339e285abd56ab21685ae31d49b41d7e Mon Sep 17 00:00:00 2001 From: Gabriel Jablonski Date: Thu, 30 Apr 2026 16:25:45 -0300 Subject: [PATCH] feat(super-admin): hide assignee tabs for basic agents (#279) * fix(featurable): backport feature_flag_value helper from chatwoot-pro-main Adds the two's-complement-aware helper that returns a signed bigint-safe value for SQL queries against the feature_flags column. Mirrors the existing helper in chatwoot-pro-main so future backports of pro features that reference it (e.g. kanban filters) compile cleanly on main. Note: the helper does NOT fix FlagShihTzu's write path; new account-level toggles should use account.settings jsonb instead of feature_flags (see AGENTS.md). Co-Authored-By: Claude Opus 4.7 (1M context) * feat(super-admin): toggle to hide assignee tabs for basic agents Adds two account-level settings, configurable from the super admin dashboard, that hide the "Unassigned" and "All" tabs of the conversation list for users with the basic agent role (admins and custom roles are unaffected). Hiding "Unassigned" implicitly hides "All", since seeing the full queue without the unassigned subset is incoherent. The constraint is enforced both in the backend (before_validation forces hide_agent_all_tab=true when hide_agent_unassigned_tab is on) and in the super admin form (the "All" checkbox is disabled and auto-checked when "Unassigned" is checked). Storage uses account.settings (jsonb) instead of feature_flags to sidestep the bigint bit-position overflow that happens once features.yml crosses 64 entries, and to keep keys stable across the main and chatwoot-pro-main forks where feature bit positions diverge. AGENTS.md documents the rationale and the recipe to add future toggles. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(chat-list): guard activeAssigneeTabCount against missing tab When the visibility settings hide the currently selected tab, the fallback watch resets activeAssigneeTab to ME, but activeAssigneeTabCount re-evaluates in the same reactive cycle and can read .count on undefined before the watch flushes. Use optional chaining + nullish fallback so the count safely returns 0 during the brief inconsistency. --------- Co-authored-by: Claude Opus 4.7 (1M context) --- AGENTS.md | 15 +++++++ app/dashboards/account_dashboard.rb | 8 +++- app/fields/hide_agent_all_tab_field.rb | 4 ++ .../dashboard/components/ChatList.vue | 38 +++++++++++++--- app/models/account.rb | 15 +++++++ .../concerns/account_settings_schema.rb | 2 + app/models/concerns/featurable.rb | 10 +++++ .../hide_agent_all_tab_field/_form.html.erb | 25 +++++++++++ .../hide_agent_all_tab_field/_index.html.erb | 1 + .../hide_agent_all_tab_field/_show.html.erb | 1 + spec/models/account_spec.rb | 44 +++++++++++++++++++ spec/models/concerns/featurable_spec.rb | 28 ++++++++++++ 12 files changed, 185 insertions(+), 6 deletions(-) create mode 100644 app/fields/hide_agent_all_tab_field.rb create mode 100644 app/views/fields/hide_agent_all_tab_field/_form.html.erb create mode 100644 app/views/fields/hide_agent_all_tab_field/_index.html.erb create mode 100644 app/views/fields/hide_agent_all_tab_field/_show.html.erb create mode 100644 spec/models/concerns/featurable_spec.rb diff --git a/AGENTS.md b/AGENTS.md index 97a940410..3f481d5cf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -115,3 +115,18 @@ Practical checklist for any change impacting core logic or public APIs ## Branding / White-labeling note - For user-facing strings that currently contain "Chatwoot" but should adapt to branded/self-hosted installs, prefer applying `replaceInstallationName` from `shared/composables/useBranding` in the UI layer (for example tooltip and suggestion labels) instead of adding hardcoded brand-specific copy. + +## Account-level toggles: do NOT extend `config/features.yml` + +- `Account#feature_flags` is a `bigint` driven by FlagShihTzu, with each YAML entry mapped to bit position `index` (0-based). Signed bigint can only hold bits 0..63. Adding a 65th entry produces values >= 2^64 that overflow the column on write and silently break high-bit features. +- `chatwoot-pro-main` already inserts `kanban` and `internal_chat_pro` mid-list, pushing upstream features to bits 60+. After merging into Pro, any new flag added on `main` lands at an even higher bit, accelerating the overflow. The `Featurable.feature_flag_value` helper applies a two's-complement workaround that only fixes manual SQL queries (`feature_flags & ? != 0`); it does NOT fix the FlagShihTzu write path used by the superadmin form. +- Local DB pitfall: bit positions differ between `main` and `chatwoot-pro-main` because of the kanban/internal_chat_pro insertion. The same bit set on one branch maps to a different feature on the other. Use separate dev DBs per branch or reset `feature_flags` when switching. + +For NEW account-level toggles, prefer the `settings` jsonb column instead of `feature_flags`: + +1. Declare a `store_accessor :settings, :your_toggle` in `app/models/account.rb` and override the writer to cast (`super(ActiveModel::Type::Boolean.new.cast(value))` for booleans) so JSON schema validation accepts the value. +2. Add the key to `SETTINGS_PARAMS_SCHEMA` in `app/models/concerns/account_settings_schema.rb`. +3. Register it as a `Field::Boolean` (or appropriate field) in `app/dashboards/account_dashboard.rb` (`ATTRIBUTE_TYPES`, `FORM_ATTRIBUTES`, `SHOW_PAGE_ATTRIBUTES`). +4. The frontend reads it from `account.settings.your_toggle` (already serialized via `app/views/api/v1/models/_account.json.jbuilder` as `json.settings resource.settings`). + +This keeps toggles keyed by name (immune to bit-position drift between branches) and unbounded by the bigint width. diff --git a/app/dashboards/account_dashboard.rb b/app/dashboards/account_dashboard.rb index 9be674f11..a184a1df8 100644 --- a/app/dashboards/account_dashboard.rb +++ b/app/dashboards/account_dashboard.rb @@ -34,7 +34,9 @@ class AccountDashboard < Administrate::BaseDashboard locale: Field::Select.with_options(collection: LANGUAGES_CONFIG.map { |_x, y| y[:iso_639_1_code] }), status: Field::Select.with_options(collection: [%w[Active active], %w[Suspended suspended]]), account_users: Field::HasMany, - custom_attributes: Field::String + custom_attributes: Field::String, + hide_agent_unassigned_tab: Field::Boolean, + hide_agent_all_tab: HideAgentAllTabField }.merge(enterprise_attribute_types).freeze # COLLECTION_ATTRIBUTES @@ -70,6 +72,8 @@ class AccountDashboard < Administrate::BaseDashboard status conversations account_users + hide_agent_unassigned_tab + hide_agent_all_tab ] + enterprise_show_page_attributes).freeze # FORM_ATTRIBUTES @@ -87,6 +91,8 @@ class AccountDashboard < Administrate::BaseDashboard name locale status + hide_agent_unassigned_tab + hide_agent_all_tab ] + enterprise_form_attributes).freeze # COLLECTION_FILTERS diff --git a/app/fields/hide_agent_all_tab_field.rb b/app/fields/hide_agent_all_tab_field.rb new file mode 100644 index 000000000..2ac6e186a --- /dev/null +++ b/app/fields/hide_agent_all_tab_field.rb @@ -0,0 +1,4 @@ +require 'administrate/field/base' + +class HideAgentAllTabField < Administrate::Field::Boolean +end diff --git a/app/javascript/dashboard/components/ChatList.vue b/app/javascript/dashboard/components/ChatList.vue index 779eff58a..11bc063ec 100644 --- a/app/javascript/dashboard/components/ChatList.vue +++ b/app/javascript/dashboard/components/ChatList.vue @@ -63,6 +63,7 @@ import { } from '../store/modules/conversations/helpers/actionHelpers'; import { getUserPermissions, + getUserRole, filterItemsByPermission, } from 'dashboard/helper/permissionsHelper.js'; import { matchesFilters } from '../store/modules/conversations/helpers/filterHelpers'; @@ -129,6 +130,7 @@ const inboxesList = useMapGetter('inboxes/getInboxes'); const campaigns = useMapGetter('campaigns/getAllCampaigns'); const labels = useMapGetter('labels/getLabels'); const currentAccountId = useMapGetter('getCurrentAccountId'); +const getAccount = useMapGetter('accounts/getAccount'); // We can't useFunctionGetter here since it needs to be called on setup? const getTeamFn = useMapGetter('teams/getTeam'); const getConversationById = useMapGetter('getConversationById'); @@ -197,9 +199,29 @@ const userPermissions = computed(() => { return getUserPermissions(currentUser.value, currentAccountId.value); }); +const assigneeTabPermissions = computed(() => { + if (getUserRole(currentUser.value, currentAccountId.value) !== 'agent') { + return ASSIGNEE_TYPE_TAB_PERMISSIONS; + } + + const accountSettings = + getAccount.value(currentAccountId.value)?.settings || {}; + const hideUnassigned = Boolean(accountSettings.hide_agent_unassigned_tab); + const hideAll = hideUnassigned || Boolean(accountSettings.hide_agent_all_tab); + + if (!hideUnassigned && !hideAll) return ASSIGNEE_TYPE_TAB_PERMISSIONS; + + const { unassigned, all, ...rest } = ASSIGNEE_TYPE_TAB_PERMISSIONS; + return { + ...rest, + ...(hideUnassigned ? {} : { unassigned }), + ...(hideAll ? {} : { all }), + }; +}); + const assigneeTabItems = computed(() => { return filterItemsByPermission( - ASSIGNEE_TYPE_TAB_PERMISSIONS, + assigneeTabPermissions.value, userPermissions.value, item => item.permissions ).map(({ key, count: countKey }) => ({ @@ -242,10 +264,10 @@ const conversationCustomAttributes = useFunctionGetter( ); const activeAssigneeTabCount = computed(() => { - const count = assigneeTabItems.value.find( - item => item.key === activeAssigneeTab.value - ).count; - return count; + return ( + assigneeTabItems.value.find(item => item.key === activeAssigneeTab.value) + ?.count ?? 0 + ); }); const conversationListPagination = computed(() => { @@ -856,6 +878,12 @@ onMounted(() => { watch(conversationList, subscribePresenceForTopChats); +watch(assigneeTabItems, items => { + if (!items.some(item => item.key === activeAssigneeTab.value)) { + updateAssigneeTab(wootConstants.ASSIGNEE_TYPE.ME); + } +}); + onBeforeUnmount(() => { if (presenceInterval) clearInterval(presenceInterval); }); diff --git a/app/models/account.rb b/app/models/account.rb index f228846b0..2eaffd8bd 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -52,6 +52,17 @@ class Account < ApplicationRecord store_accessor :settings, :reporting_timezone store_accessor :settings, :keep_pending_on_bot_failure store_accessor :settings, :captain_auto_resolve_mode + store_accessor :settings, :hide_agent_unassigned_tab, :hide_agent_all_tab + before_validation :enforce_agent_assignee_tabs_constraint + + def hide_agent_unassigned_tab=(value) + super(ActiveModel::Type::Boolean.new.cast(value)) + end + + def hide_agent_all_tab=(value) + super(ActiveModel::Type::Boolean.new.cast(value)) + end + include AccountCaptainAutoResolve has_many :account_users, dependent: :destroy_async @@ -190,6 +201,10 @@ class Account < ApplicationRecord errors.add(:reporting_timezone, I18n.t('errors.account.reporting_timezone.invalid')) end + def enforce_agent_assignee_tabs_constraint + self.hide_agent_all_tab = true if hide_agent_unassigned_tab + end + def validate_support_email_format value = attributes['support_email'] return if value.blank? diff --git a/app/models/concerns/account_settings_schema.rb b/app/models/concerns/account_settings_schema.rb index 52e1c2811..8de371e52 100644 --- a/app/models/concerns/account_settings_schema.rb +++ b/app/models/concerns/account_settings_schema.rb @@ -11,6 +11,8 @@ module AccountSettingsSchema 'audio_transcriptions': { 'type': %w[boolean null] }, 'auto_resolve_label': { 'type': %w[string null] }, 'keep_pending_on_bot_failure': { 'type': %w[boolean null] }, + 'hide_agent_unassigned_tab': { 'type': %w[boolean null] }, + 'hide_agent_all_tab': { 'type': %w[boolean null] }, 'captain_auto_resolve_mode': { 'type': %w[string null], 'enum': ['evaluated', 'legacy', 'disabled', nil] }, 'conversation_required_attributes': { 'type': %w[array null], diff --git a/app/models/concerns/featurable.rb b/app/models/concerns/featurable.rb index 919dddd41..1cc1b2a0c 100644 --- a/app/models/concerns/featurable.rb +++ b/app/models/concerns/featurable.rb @@ -12,6 +12,16 @@ module Featurable result[result.keys.size + 1] = "feature_#{feature['name']}".to_sym end + def self.feature_flag_value(feature_name) + feature_index = FEATURE_LIST.index { |f| f['name'] == feature_name } + return 0 if feature_index.nil? + + value = 2**feature_index + # Convert to signed 64-bit representation for PostgreSQL bigint compatibility. + # Values >= 2^63 overflow signed bigint; two's complement conversion fixes this. + value >= (1 << 63) ? value - (1 << 64) : value + end + included do include FlagShihTzu has_flags FEATURES.merge(column: 'feature_flags').merge(QUERY_MODE) diff --git a/app/views/fields/hide_agent_all_tab_field/_form.html.erb b/app/views/fields/hide_agent_all_tab_field/_form.html.erb new file mode 100644 index 000000000..ef040ab02 --- /dev/null +++ b/app/views/fields/hide_agent_all_tab_field/_form.html.erb @@ -0,0 +1,25 @@ +
+ <%= f.label field.attribute %> +
+
+ <%= f.check_box field.attribute %> + +
diff --git a/app/views/fields/hide_agent_all_tab_field/_index.html.erb b/app/views/fields/hide_agent_all_tab_field/_index.html.erb new file mode 100644 index 000000000..6d9dbc907 --- /dev/null +++ b/app/views/fields/hide_agent_all_tab_field/_index.html.erb @@ -0,0 +1 @@ +<%= field.to_s %> diff --git a/app/views/fields/hide_agent_all_tab_field/_show.html.erb b/app/views/fields/hide_agent_all_tab_field/_show.html.erb new file mode 100644 index 000000000..6d9dbc907 --- /dev/null +++ b/app/views/fields/hide_agent_all_tab_field/_show.html.erb @@ -0,0 +1 @@ +<%= field.to_s %> diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index f3bce36c9..11c6e93e9 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -244,6 +244,50 @@ RSpec.describe Account do end end + context 'when toggling agent assignee tab visibility' do + it 'casts truthy form input to boolean true' do + account.hide_agent_unassigned_tab = '1' + account.hide_agent_all_tab = 'true' + + expect(account.hide_agent_unassigned_tab).to be true + expect(account.hide_agent_all_tab).to be true + expect(account.settings['hide_agent_unassigned_tab']).to be true + expect(account.settings['hide_agent_all_tab']).to be true + end + + it 'casts falsy form input to boolean false' do + account.hide_agent_unassigned_tab = '0' + account.hide_agent_all_tab = 'false' + + expect(account.hide_agent_unassigned_tab).to be false + expect(account.hide_agent_all_tab).to be false + end + + it 'persists across save with the schema validator passing' do + account.update!(hide_agent_unassigned_tab: '0', hide_agent_all_tab: '1') + reloaded = described_class.find(account.id) + + expect(reloaded.hide_agent_unassigned_tab).to be false + expect(reloaded.hide_agent_all_tab).to be true + end + + it 'rejects non-boolean values via the JSON schema validator' do + account.settings = { hide_agent_unassigned_tab: 'maybe' } + expect(account).to be_invalid + expect(account.errors.messages).to have_key(:hide_agent_unassigned_tab) + end + + it 'forces hide_agent_all_tab to true when hide_agent_unassigned_tab is enabled' do + account.update!(hide_agent_unassigned_tab: true, hide_agent_all_tab: false) + expect(account.reload.hide_agent_all_tab).to be true + end + + it 'leaves hide_agent_all_tab untouched when hide_agent_unassigned_tab is false' do + account.update!(hide_agent_unassigned_tab: false, hide_agent_all_tab: false) + expect(account.reload.hide_agent_all_tab).to be false + end + end + context 'when using with_auto_resolve scope' do it 'finds accounts with auto_resolve_after set' do account.update!(auto_resolve_after: 40 * 24 * 60) diff --git a/spec/models/concerns/featurable_spec.rb b/spec/models/concerns/featurable_spec.rb new file mode 100644 index 000000000..2e232d546 --- /dev/null +++ b/spec/models/concerns/featurable_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Featurable do + describe '.feature_flag_value' do + it 'returns 0 for unknown features' do + expect(described_class.feature_flag_value('nonexistent_feature')).to eq(0) + end + + it 'returns the unsigned bit value for low-position features' do + first_feature = described_class::FEATURE_LIST.first['name'] + expect(described_class.feature_flag_value(first_feature)).to eq(1) + end + + it 'returns the signed two\'s complement representation for high-position features' do + stub_const( + "#{described_class}::FEATURE_LIST", + Array.new(64) { |i| { 'name' => "feature_#{i}" } }.freeze + ) + + # Position 64 (0-indexed 63) is the sign bit on signed bigint. + expect(described_class.feature_flag_value('feature_63')).to eq(-(1 << 63)) + # Lower positions stay positive. + expect(described_class.feature_flag_value('feature_62')).to eq(1 << 62) + end + end +end