diff --git a/frontend/layouts/sidebar.html b/frontend/layouts/sidebar.html index 7367880b5..577a4119e 100644 --- a/frontend/layouts/sidebar.html +++ b/frontend/layouts/sidebar.html @@ -491,11 +491,32 @@
Messaging -
  • - - sales channels - Sales Channels +
  • + + online ordering + Online Ordering + +
    • +
  • + + kiosk machine + Kiosk Machine + +
    • +
  • + + delivery partners + Delivery Partners + +
    • +
  • + + webshop + Webshop
  • diff --git a/frontend/modules/settings_write.html b/frontend/modules/settings_write.html index 369c2d26c..41a6d7030 100644 --- a/frontend/modules/settings_write.html +++ b/frontend/modules/settings_write.html @@ -83,10 +83,25 @@

    Messaging - + Online Ordering + + - Sales Channels + Kiosk Machine + + + Delivery Partners + + + Webshop Serving periods + +
    +
    +
    Hotels and other venues
    +
    +
    + +
    Hotels and other venues
    + + Buildings you deliver into, each with its own QR codes. The markup is what the guest pays on top; the commission is what you owe the venue. They are separate numbers. + +
    + + +
    + +
    +
    +
    +
    +
    +
    Delivery and other charges
    +
    +
    + +
    Delivery and other charges
    + + Charged on top of the food. Free above is the order value that waives the fee; minimum is the smallest order you will send out at all. + +
    + +
    + +
    +
    +
    @@ -4846,214 +4922,44 @@
    -
    + +
    -
    Channels and ordering
    +
    Online Ordering
    -
    -
    +
    +
    +
    +
    Default branch
    + + The store address a plain /order link means. Leave empty if you have one branch. + +
    +
    + + +
    +
    + + +
    New online orders
    + + Send straight to the kitchen, or hold each order until somebody accepts it. Holding raises an alarm on the till so a waiting order is not missed. + +
    +
    + + +
    +
    +
    +
    + +
    +
    +
    +
    +
    +
    +
    + + + + + + + + +
    +
    +
    +
    +
    +
    Kiosk Machine
    +
    +
    + + The store id, images, payment methods and opening hours are the same ones online ordering uses, so they are set once under Online Ordering. Only the printer is the machine's own. + + +
    +
    + +
    +
    +
    +
    +
    +
    Delivery Partners
    +
    +
    + +
    +
    +
    +
    +
    +
    Partners
    + + Aggregators and webshops that send you orders. The commission is what they keep, so a report can show what you actually earned. + + +
    + +
    + + +
    +
    + +
    +
    +
    +
    +
    +
    +
    +
    +
    +
    +
    +
    +
    + + +
    +
    +
    +
    +
    +
    Webshop
    +
    +
    + +
    +
    +
    +
    +
    + + A shop you run yourself - OpenCart, WooCommerce, your own site. The commission is what the platform keeps, so a report can show what you actually earned. + +
    + +
    +
    + +
    +
    +
    +
    +
    +
    +
    +
    +
    +
    +
    +
    +
    + + + + +
    ' + - '
    ' + - '' + - '' + - '
    '; - }); - $('#sales_channels_list').html(html); - }, + /* + * CHANNELS stays, renderChannels went. + * + * The list is still the vocabulary - partnerRow labels its kinds from it - + * but a shop no longer ticks channels on a settings screen. The Features + * page decides which channels exist; a second set of checkboxes behind it + * was the duplicate that made this page feel like one big "Sales Channels" + * screen in the first place. + */ partnerRow: function (partner) { var self = PosnicPro.salesChannels; @@ -7997,10 +7996,25 @@ PosnicPro.salesChannels = { '
    '; }, + /* + * ONE list of partners, drawn into two screens. + * + * A partner row already says which kind it is - an aggregator that + * delivers, or a webshop the shop runs itself - so Delivery Partners and + * Webshop are two views of the same stored list rather than two lists to + * keep in step. collect() reads `.channel-partner-row` wherever it sits, + * so a save from either screen still writes both. + */ renderPartners: function (partners) { var self = PosnicPro.salesChannels; var list = Array.isArray(partners) ? partners : []; - $('#sales_channel_partner_rows').html(list.map(self.partnerRow).join('')); + var apps = []; + var shops = []; + list.forEach(function (p) { + (String((p || {}).channel) === 'ecommerce' ? shops : apps).push(p); + }); + $('#sales_channel_partner_rows').html(apps.map(self.partnerRow).join('')); + $('#webshop_partner_rows').html(shops.map(self.partnerRow).join('')); }, /* @@ -8129,7 +8143,6 @@ PosnicPro.salesChannels = { var self = PosnicPro.salesChannels; PosnicPro.get({ url: 'settings/group/channels', data: {} }, function (response) { var values = (response && response.data && response.data.values) || {}; - self.renderChannels(values.sales_channels_enabled); self.renderPartners(values.sales_channel_partners); self.renderVenues(values.partner_venues); self.renderCharges(values.channel_charges); @@ -8151,7 +8164,6 @@ PosnicPro.salesChannels = { }, function () { /* A shop that has never saved these has nothing stored yet, which is not an error. Draw the till, which every shop has. */ - self.renderChannels(['pos']); self.renderPartners([]); self.renderVenues([]); self.renderCharges({}); @@ -8159,9 +8171,6 @@ PosnicPro.salesChannels = { }, collect: function () { - var enabled = []; - $('.sales-channel-box:checked').each(function () { enabled.push($(this).val()); }); - var partners = []; $('.channel-partner-row').each(function () { var $row = $(this); @@ -8214,7 +8223,17 @@ PosnicPro.salesChannels = { }); return { - sales_channels_enabled: enabled, + /* + * sales_channels_enabled is DELIBERATELY ABSENT, for exactly the + * reason menu_dayparts is below. + * + * The "ways this shop takes orders" checkboxes were a second place + * to choose channels after the Features cards, so they went. Their + * markup is gone, which means collecting the key here would send an + * empty list and quietly wipe whatever a shop had chosen - the + * item channel picker reads it. The group endpoint writes only + * what it is given, so leaving it out keeps the stored value. + */ sales_channel_partners: partners, /* Empty is a real answer: it means "work it out", which is right for the one-branch shops that are most of them. */ @@ -8281,15 +8300,85 @@ $(document).on('click', '#add_channel_partner', function () { $('#sales_channel_partner_rows').append(PosnicPro.salesChannels.partnerRow({})); }); +$(document).on('click', '#add_webshop_partner', function () { + $('#webshop_partner_rows').append(PosnicPro.salesChannels.partnerRow({ channel: 'ecommerce' })); +}); + +/* Change a row's kind and it belongs on the other screen. Moving it there is + the honest answer: leaving an "Own webshop" row sitting under Delivery + Partners is how a shop ends up believing it saved something it cannot find. */ +$(document).on('change', '.partner-channel', function () { + var $row = $(this).closest('.channel-partner-row'); + var target = $(this).val() === 'ecommerce' ? '#webshop_partner_rows' : '#sales_channel_partner_rows'; + if (!$row.parent().is(target)) { $row.appendTo(target); } +}); + $(document).on('click', '.remove-channel-partner', function () { $(this).closest('.channel-partner-row').remove(); }); -$(document).on('submit', '#sales_channels_form', function (e) { - e.preventDefault(); +/* + * EVERY channel screen saves the whole group, and that is deliberate. + * + * collect() reads the DOM by class, and the panes are hidden rather than + * removed, so it sees every row wherever it sits. That means a save from the + * webshop screen still writes the aggregators, the venues and the charges + * exactly as they stand - which is what the one big form did before the split. + * Collecting only the open pane would be the change that quietly wipes the + * others, which is the bug this codebase has already paid for twice. + */ +$(document).on( + 'submit', + '#online_orders_form, #delivery_partners_form, #webshop_partners_form', + function (e) { + e.preventDefault(); + return PosnicPro.salesChannels.save(); + } +); + +/* Venues and delivery charges sit on the Restaurant page, as cards with their + own Save rather than a form - the same shape the serving periods use. */ +$(document).on('click', '.save-channel-settings', function () { return PosnicPro.salesChannels.save(); }); +/* + * WHICH PRODUCTS THIS CHANNEL SELLS, shown inside the channel you are in. + * + * One copy of the screen, borrowed by whichever pane is open. Four copies + * would mean four sets of the same ids, and a duplicate id is how a screen + * starts writing to the wrong form - this page already carries one such + * landmine and does not need three more. + */ +PosnicPro.salesChannels.PANE_CHANNEL = { + 'v-pills-onlineordering': 'online', + 'v-pills-kioskmachine': 'kiosk', + 'v-pills-deliverypartners': 'marketplace', + 'v-pills-webshop': 'ecommerce' +}; + +PosnicPro.salesChannels.lendProducts = function (paneId) { + var channel = PosnicPro.salesChannels.PANE_CHANNEL[paneId]; + if (!channel) { return; } + var $host = $('#' + paneId + ' .channel-products-host').first(); + var $block = $('#channel_items_block'); + if (!$host.length || !$block.length) { return; } + if (!$block.parent().is($host)) { $block.appendTo($host); } + /* Preselected, because a shopkeeper who opened Kiosk Machine is asking + about the kiosk. The picker still lets them look at another one. */ + if ($('#channel_items_channel').length) { + $('#channel_items_channel').val(channel).trigger('change'); + } +}; + +$(document).on( + 'shown.bs.tab', + '#v-pills-onlineordering-tab, #v-pills-kioskmachine-tab, #v-pills-deliverypartners-tab, #v-pills-webshop-tab', + function () { + PosnicPro.salesChannels.lendProducts(String($(this).attr('href') || '').replace('#', '')); + } +); + /* * Serving periods: breakfast, lunch, dinner. * diff --git a/languages/_english.json b/languages/_english.json index a36d1c894..3aa25252b 100644 --- a/languages/_english.json +++ b/languages/_english.json @@ -92,6 +92,7 @@ "lang_add_to_or_remove_from_stock_for_many_items": "Add to or remove from stock for many items at once. Every change to a stock-tracked item is kept in its stock log, along with your note.", "lang_add_to_sale": "Add to sale", "lang_add_venue": "Add venue", + "lang_add_webshop": "Add webshop", "lang_add_whatsapp_template": "Add WhatsApp Template", "lang_add_your_employees": "Add your employees", "lang_add_your_first_expense_with_the_new_button": "Add your first expense with the New button above.", @@ -365,9 +366,6 @@ "lang_channel_pos": "Point of sale", "lang_channel_tableside": "Captain app", "lang_channel_whatsapp": "WhatsApp", - "lang_channels_and_products": "Channels and products", - "lang_channels_in_use": "Ways this shop takes orders", - "lang_channels_settings": "Channels and ordering", "lang_charge_amount": "Charge amount", "lang_charge_label": "Charge label", "lang_charge_name_e_g_parcel": "Charge name (e.g. Parcel)", @@ -704,6 +702,7 @@ "lang_deletetaxrecord_title": "Delete Tax Record", "lang_delivery_and_fees": "Delivery and other charges", "lang_delivery_and_fees_help": "Charged on top of the food. Free above is the order value that waives the fee; minimum is the smallest order you will send out at all.", + "lang_delivery_apps": "Apps", "lang_demodata_hint": "The sample records your shop can try itself with - choose a trade, reset for a fresh set, or switch the feature off to remove them.", "lang_demopack_help": "Your shop was set up with one trade's sample products. Pick a different trade and we will swap them for that one.", "lang_demopack_install": "Install this trade's samples", @@ -1340,6 +1339,7 @@ "lang_kiosk_printer": "Kiosk printer", "lang_kiosk_printer_help": "Only the self-service machine in your shop uses this. Everything else on this page is your storefront, which the machine and your customers' phones both show.", "lang_kiosk_report": "Kiosk Report", + "lang_kiosk_shares_storefront": "The store id, images, payment methods and opening hours are the same ones online ordering uses, so they are set once under Online Ordering. Only the printer is the machine's own.", "lang_kot_report": "KOT Report", "lang_kot_reports": "KOT Reports", "lang_kot_title": "KOT", @@ -1510,7 +1510,6 @@ "lang_module_access": "Module access", "lang_module_captain": "Captain App", "lang_module_captain_desc": "Your staff taking orders at the table on a phone, straight to the kitchen.", - "lang_module_channels": "Sales Channels", "lang_module_credit": "Customer Credit", "lang_module_delivery_partners": "Delivery Partners", "lang_module_delivery_partners_desc": "Swiggy, Zomato and the rest. Record what each one keeps so a report can show what you actually earned.", @@ -2996,6 +2995,8 @@ "lang_way2sms": "Way2SMS", "lang_webhook": "Webhook", "lang_webhook_url_paste_your_razorpay_setting": "Webhook url paste your Razorpay setting.", + "lang_webshop_rows_help": "A shop you run yourself - OpenCart, WooCommerce, your own site. The commission is what the platform keeps, so a report can show what you actually earned.", + "lang_webshops": "Webshops", "lang_website_title": "Website", "lang_wednesday": "Wednesday", "lang_weekly": "Weekly", @@ -3013,6 +3014,7 @@ "lang_wf_planshift": "Plan a shift", "lang_wf_refresh": "Refresh", "lang_what_40_means": "What 40% means", + "lang_what_it_sells": "What it sells", "lang_what_this_role_is_for": "What this role is for", "lang_what_this_user_may_do_at_the_till_without": "(what this user may do at the till without a manager)", "lang_whatsapp": "WhatsApp", diff --git a/tests/channel-panes-split.test.js b/tests/channel-panes-split.test.js new file mode 100644 index 000000000..3931cd2a1 --- /dev/null +++ b/tests/channel-panes-split.test.js @@ -0,0 +1,229 @@ +'use strict'; + +/** + * Each channel gets its own settings page, named the way a shopkeeper names it. + * + * WHAT THIS EXISTS FOR. + * + * The Features page was split into one card per channel - Online Ordering, + * Kiosk Machine, Delivery Partners, Webshop - and shipped. The settings behind + * those cards were not. All five cards pointed at the same old `#/settings/kiosk` + * page, still headed "Sales Channels", and the code said so out loud: + * + * All five point at the one channels page for now. + * The Features page is split; the settings pages behind it are not, yet. + * + * Which is honest, and completely invisible from the outside. The owner opened + * the console, saw the same combined page he had asked us to take apart, and + * had no way to tell a half-finished split from a deploy that had not landed. + * A comment is not a signal; a failing test is. + * + * So this pins the finished shape: every channel has its own pane, its own + * sidebar entry, its own module gate, and no two of them share a page. It also + * pins the wipe guard that the split made necessary. + */ + +const test = require('node:test'); +const assert = require('node:assert'); +const fs = require('node:fs'); +const path = require('node:path'); + +const ROOT = path.join(__dirname, '..'); +const read = (...p) => fs.readFileSync(path.join(ROOT, ...p), 'utf8'); + +const SETTINGS_JS = read('frontend', 'static', 'script', 'js', 'modules', 'js', 'settings.js'); +const SETTINGS_HTML = read('frontend', 'modules', 'settings_write.html'); +const SIDEBAR = read('frontend', 'layouts', 'sidebar.html'); +const CORE_JS = read('frontend', 'static', 'script', 'js', 'core', 'PosnicPro.js'); + +/** + * The channels that earn their own page, and the section each one owns. + * + * The Captain app is deliberately absent: it configures the tables it runs on, + * so its card points at Restaurant. That is a shared page on purpose, which is + * why the "no two channels share a page" rule below is written over this list + * rather than over every entry in FEATURE_HOME. + */ +const CHANNELS = [ + { module: 'module_online_ordering_enable', section: 'onlineordering' }, + { module: 'module_kiosk_enable', section: 'kioskmachine' }, + { module: 'module_delivery_partners_enable', section: 'deliverypartners' }, + { module: 'module_webshop_enable', section: 'webshop' }, +]; + +/** [module key, route key] for every feature card that names a home. */ +function featureHomes() { + const block = SETTINGS_JS.match(/PosnicPro\.settings\.FEATURE_HOME = \{([\s\S]*?)\n\};/); + assert.ok(block, 'FEATURE_HOME is no longer a literal this test can read'); + return [...block[1].matchAll(/(\w+):\s*\['([\w-]+)'/g)].map((m) => [m[1], m[2]]); +} + +/** Every pane id the settings markup actually defines. */ +function panes() { + return new Set([...SETTINGS_HTML.matchAll(/id="v-pills-([a-z0-9-]+)"/g)].map((m) => m[1])); +} + +test('every channel has a settings pane of its own', () => { + const defined = panes(); + const missing = CHANNELS.filter((c) => !defined.has(c.section)).map((c) => c.section); + assert.deepStrictEqual( + missing, + [], + `these channels have a feature card but no settings pane: ${missing.join(', ')}` + ); +}); + +test('no two channels are sent to the same page', () => { + /* The exact failure this replaces: all five cards pointed at 'kiosk', so + opening any of them showed the same combined screen. */ + const homes = new Map(featureHomes()); + const seen = new Map(); + const shared = []; + + for (const { module } of CHANNELS) { + const where = homes.get(module); + assert.ok(where, `${module} has no Configure link, so its card leads nowhere`); + if (seen.has(where)) shared.push(`${seen.get(where)} and ${module} both open #/settings/${where}`); + seen.set(where, module); + } + + assert.deepStrictEqual(shared, [], shared.join('\n ')); +}); + +test('each channel card opens the channel it names', () => { + const homes = new Map(featureHomes()); + const wrong = CHANNELS.filter(({ module, section }) => homes.get(module) !== section).map( + ({ module, section }) => `${module} -> ${homes.get(module)}, expected ${section}` + ); + assert.deepStrictEqual(wrong, [], wrong.join('\n ')); +}); + +test('no feature card points at a section that does not exist', () => { + /* + * The generic form of the same bug, and the reason the old code pointed all + * five at `kiosk` rather than at the pages it wanted: openSection falls back + * to Core Settings for a key it does not know, so a card aimed at a missing + * pane takes somebody somewhere wrong with nothing to say it went wrong. + */ + const defined = panes(); + const dangling = featureHomes() + .filter(([, where]) => !defined.has(where)) + .map(([module, where]) => `${module} -> #/settings/${where}`); + + assert.deepStrictEqual( + dangling, + [], + `a feature's Configure link lands on Core Settings instead:\n ${dangling.join('\n ')}` + ); +}); + +test('each channel has its own entry in the Manage sidebar', () => { + const missing = CHANNELS.filter((c) => !SIDEBAR.includes(`id="manage_sec_${c.section}"`)).map( + (c) => c.section + ); + assert.deepStrictEqual(missing, [], `no sidebar entry for: ${missing.join(', ')}`); +}); + +test('each channel entry follows its own feature switch', () => { + /* + * Not the derived roof. module_channels_enable is true when ANY channel is + * on, so gating the four entries on it would show a shop the Webshop page + * because it turned the kiosk on. + */ + const wrong = []; + for (const { module, section } of CHANNELS) { + const sidebarGate = new RegExp(`#manage_li_${section}'\\)\\.toggle\\(on\\('${module}'\\)`); + const pillGate = new RegExp(`#v-pills-${section}-tab'\\)\\.toggle\\(on\\('${module}'\\)`); + if (!sidebarGate.test(CORE_JS)) wrong.push(`sidebar entry ${section} is not gated on ${module}`); + if (!pillGate.test(SETTINGS_JS)) wrong.push(`pill ${section} is not gated on ${module}`); + } + assert.deepStrictEqual(wrong, [], wrong.join('\n ')); +}); + +test('the old combined page is gone, and its address still works', () => { + assert.ok( + !/id="v-pills-kiosk"/.test(SETTINGS_HTML), + 'the combined Sales Channels pane is still in the markup' + ); + assert.ok( + !/lang_module_channels">Sales Channels/.test(SIDEBAR), + '"Sales Channels" is still the name of a sidebar entry; it is our reporting word, not a shopkeeper\'s' + ); + /* Old bookmarks and the desktop app still say #/settings/kiosk. Landing them + on Core Settings would read as the page being broken. */ + assert.match( + SETTINGS_JS, + /kiosk: 'onlineordering'/, + '#/settings/kiosk no longer routes anywhere, so every old link lands on Core Settings' + ); +}); + +test('the split cannot wipe the channels a shop chose', () => { + /* + * THE TRAP THE SPLIT SET, and the third time this codebase has met it. + * + * The "ways this shop takes orders" checkboxes went, because the Features + * cards are where channels are chosen now. collect() read those checkboxes + * into `sales_channels_enabled`. Left alone, every save would have posted an + * empty list and cleared the value that decides which channels an item can + * be taken off - silently, the way menu_dayparts nearly was. + * + * The group endpoint writes only the keys it is given, so the fix is to send + * none. This asserts the key stays unsent while its markup is absent. + */ + const hasCheckboxes = /class="[^"]*sales-channel-box/.test(SETTINGS_HTML); + const collects = /sales_channels_enabled:/.test(SETTINGS_JS); + + assert.strictEqual( + hasCheckboxes, + collects, + hasCheckboxes + ? 'the channel checkboxes are back but the save no longer sends them, so the screen cannot save' + : 'collect() still sends sales_channels_enabled, but nothing draws those checkboxes - every save posts an empty list and wipes the stored value' + ); +}); + +test('a save from any channel screen still writes the whole group', () => { + /* + * The panes are hidden, not removed, and collect() reads the DOM by class - + * so one save writes every channel's rows wherever they sit. That is what + * keeps the split from losing data, and it only holds while every screen + * goes through the one save. + */ + const forms = ['online_orders_form', 'delivery_partners_form', 'webshop_partners_form']; + const bound = SETTINGS_JS.match(/\$\(document\)\.on\(\s*'submit',\s*'([^']+)'/g) || []; + const all = bound.join(' '); + const unbound = forms.filter((f) => !all.includes(f)); + assert.deepStrictEqual( + unbound, + [], + `these channel forms have no save handler: ${unbound.join(', ')}` + ); +}); + +test('venues and delivery charges did not get buried in one channel', () => { + /* + * A hotel is reachable from the QR code, the phone and every aggregator, and + * a delivery charge belongs to HOW the food travels rather than to which app + * the order came through. Both sit with the serving periods on Restaurant. + */ + const restaurant = SETTINGS_HTML.slice( + SETTINGS_HTML.indexOf('id="v-pills-tableorder"'), + SETTINGS_HTML.indexOf('id="v-pills-onlineordering"') + ); + assert.ok(restaurant.length > 0, 'the Restaurant pane no longer precedes the channel panes'); + assert.match(restaurant, /id="partner_venue_rows"/, 'venues are not on the Restaurant page'); + assert.match(restaurant, /id="channel_charge_rows"/, 'delivery charges are not on the Restaurant page'); +}); + +test('the products screen exists once, and every channel can borrow it', () => { + /* Four copies would mean four sets of the same ids, and a duplicate id is + how a screen starts writing to the wrong form. */ + const copies = (SETTINGS_HTML.match(/id="channel_items_rows"/g) || []).length; + assert.strictEqual(copies, 1, `the products screen is duplicated ${copies} times`); + + const hosts = (SETTINGS_HTML.match(/class="channel-products-host"/g) || []).length; + assert.strictEqual(hosts, CHANNELS.length, `${hosts} panes can show the products screen, wanted ${CHANNELS.length}`); + + assert.match(SETTINGS_JS, /lendProducts/, 'nothing moves the products screen into the open pane'); +});