Barkeep - #49
Merged
Merged
Conversation
* Initial plan * Add skip_approval feature to allow auto-accepting orders Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com> * Fix TypeScript errors in SettingsTab Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>
* Initial plan * Fix order cancellation WebSocket issue by parsing orderId as integer Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>
* Initial plan * Add alphabetical and category sorting to drinks pages Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>
* Initial plan * Fix guest recipe visibility in favourites endpoint Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>
* Initial plan * Add recipe toggle to pending orders screen for bartenders Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com> * Change recipe toggle to global show/hide for all orders Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>
* Initial plan * Add image crop/resize/reposition feature for drinks Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>
* Initial plan * Fix image crop/zoom/position display in DrinkForm, MenuTab, and random drink modal Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com> * Fix image crop/zoom/position in RandomDrinkModal, RecipeView, and PastOrders Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>
* Initial plan * Fix image preview CSS - remove conflicting objectPosition property Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com> * Use proper CSS transform with translate for image cropping Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com> * Fix ImageCropper to sync state with props on subsequent opens Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com> * Revert to transformOrigin approach - wrapper div method didn't work Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com> * Fix crop display issues and reset crop on new image upload Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mtaanquist <12089874+mtaanquist@users.noreply.github.com>
Serve API, WebSocket, uploads and frontend from one origin. Migrations run in-process at startup, replacing the one-shot db-init container that made Dockge report the stack as exited. Adds healthchecks, a multi-arch GHCR publish workflow, and PUID/PGID support. Verified against a copy of the production database: no schema drift, 4 to 6 migrations applied cleanly, 155 drinks / 2 bars / 36 orders intact, all 136 images serving.
* Restrict direct access to the container Bind the published port to 127.0.0.1 so only a reverse proxy on the same host can reach the app. It was previously published on all interfaces, leaving it reachable from the LAN and bypassable — which matters because QR code URLs are derived from request headers, so a request that skips the proxy could set those headers itself. Scope `trust proxy` accordingly. It was unconditionally `true`, trusting X-Forwarded-* from any source. The default now trusts only loopback and private addresses, which covers Caddy on the host reaching the container over the Docker bridge as well as a containerised Caddy on a shared network, while ignoring the headers from any public source. TRUST_PROXY overrides it with any value Express accepts. Document the shared-external-network setup for when Caddy moves into a container of its own, as a separate stack rather than a service here. Closes #29 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Shorten comments and move the details to the README Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Records the conventions settled on so far: writing style, the one-origin shape and why it is that way, how migrations work, the branch and release flow, and what must never be committed. Applies the writing rule to the comments added with the single-image work, which had grown into paragraphs. The reasoning they carried is preserved in the commit messages and pull requests where it belongs. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Separate building the app from starting it server.js opened the database and listened on a port as soon as it was imported, and every route module reached for one shared database handle. Nothing could be exercised without binding a real port and writing to the real database. createApp() now takes the database and folders it should use and returns the configured app, listening nowhere. Route modules take what they need instead of importing it. server.js is left as the bootstrap: open the database, build the app, listen. No behaviour change; this is what makes the tests in the next commit possible. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add test and lint tooling to both halves Vitest and supertest for the backend, plus an ESLint setup it never had. The frontend had an ESLint config in the old format while depending on ESLint 9, which only reads the new one — so linting there has never actually run. Converted it. That surfaced a backlog. Five real faults are fixed here; the rest are `any` types and empty object types that issues #27 and #31 already cover, left as warnings with a note to turn them back into errors once those land. Also drops the unused uuid dependency and quietens migration output while tests run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fix the faults linting turned up - CategoriesTab declared EditCategoryFormProps twice, verbatim. TypeScript merges identical declarations, so it compiled silently. - LandingPage declared a variable directly inside two switch cases, where it is visible to the other branches. - RecipeView called onClose with a bare && expression. - DrinkForm caught an error it never used. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Run lint and tests on every push One job with a fixed name, so branch protection has something stable to require rather than names that shift when the steps change. Documents how to run the checks locally and what the tests are for. Closes #25 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Order updates only ever travelled one way. The browser sent a single message on connecting, to say which bar it was watching; everything else was the server telling it something had changed. The old setup never recovered from a dropped connection in any useful way. It tried three times, back to back rather than spaced out — the delay it meant to wait was cancelled by its own retry — and then gave up until someone pressed a button. A phone that sleeps, a moment of bad wifi or a restart left that guest silently stuck. Browsers reconnect a one-way stream by themselves, indefinitely, so that whole problem goes away along with the code that tried to solve it. Which bar to watch is now part of the address rather than a message sent after connecting, and a dead connection cleans itself up, so the heartbeat bookkeeping is gone too. 420 lines become about 90. Closes #26 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signing out called localStorage.clear(), which wipes everything the browser has stored for the whole site, not just the bar's own settings. Now it removes only keys with the app's prefix — which also catches the activity timestamp it used to leave behind. Also shares the activity key rather than repeating the literal in two places. The duplicate interface and unused imports this issue also listed were fixed in #36, when linting first ran. The remaining `any` types are left for #27 and #37. Closes #31 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Photos were only ever removed when their drink was deleted, so two things left files behind: changing a drink's photo, and starting to add a drink and then not saving it — the photo is stored before the drink is. Replacing a photo now removes the old one, unless another drink is using the same file. The same check applies on delete, which it did not before. As a backstop, anything nothing refers to and older than a day is cleared out on start and once a day after. Recent files are left alone in case someone is still filling in the form. Run against a copy of the real photos folder, it picks out exactly the seven unused files found earlier and leaves all 136 in use untouched. Closes #32 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The API shapes now live in shared/types.d.ts and are imported by both halves, so a change on one side that the other does not follow becomes a build error rather than something a guest finds. It holds types only, so it disappears when built and neither half has to bundle it. The routes were largely the same few steps written out repeatedly: a try/catch that logged and returned 500, a lookup to check something exists, a hand-rolled list of fields to update. Those are now shared: route() turns a thrown error into the right reply, HttpError carries the status, findBar/findDrink do the lookups, buildUpdate assembles partial updates. Roughly 1,600 lines of routes became 1,100 without losing anything. Two things this turned up: - The orders report built its date range by putting a value straight from the address bar into the SQL. It is now checked and passed as a parameter. - The frontend claimed drinks had a boolean in_stock while the API sends 1 or 0. Typed honestly as Flag rather than quietly changing what is sent. Tests and their helpers are TypeScript too, which caught a live-update test sending a field the update in question does not carry. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The second half of #27. The pages were writing out their own copy of every API shape in AppContext, and the copy had drifted. They now come from shared/types.d.ts through frontend/src/types.ts, so the two sides cannot disagree without the build saying so. Three things the compiler found once the real shapes were in place: Settings lost the auto-accept toggle. Signing in replied with a few loose fields rather than the bar, so the pages pieced one together and dropped skip_approval doing it. On the QR code path a complete bar was fetched and then overwritten with the patched-up one. Sign-in now replies with the whole bar; so does creating one. Opening a drink with no recipe threw. The recipe was typed as always being there, but it is nullable, and the recipe view called .match on it. Flags were typed as booleans. They arrive as 0 or 1 and only worked by being truthy, so flipping one produced a boolean the API would not take. Also adds the analytics shapes, which were never typed at all, and a test for the sign-in reply. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Migrations were tracked by filename alone, so editing one that had already run did nothing at all — the database and the repository drifted apart with no signal. A sha256 of each file is now recorded alongside the name, and a start that finds a recorded file changed stops with a message saying which one. Existing databases have rows with no fingerprint. Rather than leave those unverifiable for good, the first start stamps them with the files as they stand. Nothing can be proved about them either way, but from then on an edit is caught like any other. Also fixes the recovery path the issue flagged. It treated "duplicate column name" as "this whole file is already applied", so a migration that both creates a table and adds a column would skip the create if only the column existed. 2025-09-10-add-categories.sql is exactly that shape. It now applies the file a statement at a time, stepping over columns that are already there, and refuses altogether unless every statement is safe to run again. runMigrations() takes the folder to read, so tests can use their own. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* Break up the guest's page CustomerInterface was 735 lines covering browsing, ordering, favourites and the surprise-me flow at once. It is 311 now, with the pieces it was made of pulled out: - useGuestMenu holds the fetching, grouping and filtering - DrinkGrid replaces the same seven-prop grid written out five times - MenuFilters holds the sidebar and the phone dropdown - OrderPlacedModal was inline RandomDrinkModal already existed as a component and was never imported — the page had its own copy of the same markup. It uses the component now. getStatusIcon and getStatusColor were written out in both CustomerInterface and OrdersTab. They live in utils/orderStatus, keyed by status so the compiler catches a missing one, and OrderStatusCard imports them rather than taking them as props. apiCall is memoised. It was rebuilt every render, so anything listing it as a dependency reloaded constantly — the live-updates connection was being torn down and reopened on every order that came in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Break up the bartender's pages and the shared state The rest of #30. AppContext 344 -> 256. The five saved settings each had their own set-and-persist wrapper written out; they share a useStoredState hook now, which also survives a corrupt saved value instead of leaving a blank page. apiCall moved to utils/api, where being a plain function keeps it stable for free. DrinkForm 496 -> 281. Adding and changing a drink sent the same body to two places; that is one call now. The picture field, which is most of the form, is its own component. AnalyticsTab 366 -> 209. Four stat cards, two bar charts and two stat lists were each written out in full; they are StatCard, RankedBars and StatList. editingDrink was Drink | {} | null, where {} meant "adding one". It is "new" now, so the type says what it means and the ten "field in drink" checks in the form are gone. The base spirits were listed in both the guest's menu and the drink form. They come from utils/spirits. Avg orders per day was totalOrders / 7 -- a made-up divisor. The server sends a real average, over the report's window, so it says which window. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Closes #37. Both halves are clean at --max-warnings 0, and the two rules that were downgraded when linting first ran are errors again. The `any` types are gone. apiCall is generic, so a caller says what it expects rather than getting something unchecked: apiCall<Drink[]>(...). The nineteen places that broke were all payloads nobody had ever typed. Two of them were API shapes with no name, so SignedIn and BarQrCode are in shared/types.d.ts and the routes return them with `satisfies`. The missing effect dependencies are fixed rather than silenced, which apiCall being stable now makes possible: the fetchers are memoised and listed, so a stale closure cannot quietly hold an old bar id. ImageCropper watches the crop's x and y rather than the object around them, which the caller rebuilds every render. useApp and useLiveUpdates, and the contexts they read, moved into hooks/. Those files hold no components, so fast refresh works on the providers again and the disable comment in LiveUpdatesContext is gone. Both lint scripts now fail on a warning, and CI runs them, so this does not quietly build back up. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The frontend had none. These run against a stand-in browser, so they need no server and no real browser, and they sit alongside the server's in tests/ under the same Vitest setup. 37 tests, weighted towards things that have actually gone wrong: - the menu only lists what is in stock, groups by spirit in the bar's order rather than alphabetically, sorts inside each group, and files a drink with no category under Uncategorized - a guest cannot order a second drink while one is on the go - cancelling asks first, and does nothing if you say no - an order that is cancelled elsewhere disappears from the list - the connection notice waits out a blip and only speaks up if it lasts - saved settings survive a refresh, blanks are removed rather than stored, an unreadable value falls back instead of blanking the page, and signing out leaves other sites' storage alone - a drink with no picture or no recipe still opens Two are regression tests for the apiCall identity bug, and both were checked by putting the old behaviour back: the menu would load in a loop, and the live connection would be dropped and reopened on every order. Test helpers: fakeApi() answers requests and records them, signIn() sets up a session, FakeEventSource stands in for live updates, and aBar/aDrink/ anOrder make something to work with. CI runs them, along with a typecheck that now covers the test files too. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
) * Fix the past orders page, and rewrite the README Found by walking the app end to end against a copy of the real data. The past orders page never loaded anything. It read whatever the menu had already put in shared state, so opening it from the menu worked, but arriving at the address directly, or refreshing it, showed an empty list. It loads its own orders now. Two tests cover it, both of which fail against the old page. The README described the app as it was three refactors ago: WebSockets, a /ws path, a port published on every interface, and migrations tracked by name alone. It now says what the bar actually does, for guests and for the host, in plain terms, and documents every setting that exists. Also removes a dead /ws entry from the dev proxy config. Nothing has spoken WebSocket since the live updates were replaced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Use a plain port in the example compose file The published port was 21000, which is one particular host's choice, not a sensible default for an example. Both sides are 3000 now, with a note that the first number is the one to change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Rename the project to Barkeep, and rewrite the README as an introduction The README read as an account of what had been wrong and what we did about it, which is no use to someone seeing this for the first time. It now opens with what the thing is, walks through how an evening actually goes, then splits the features into what guests get and what the host gets. The history is gone; the reasoning that still matters is stated as a plain fact rather than as a fix. Renamed throughout: the image is ghcr.io/mtaanquist/barkeep, the compose service and container are barkeep, the page title and the landing page say Barkeep, and the three package names follow. The one exception is the browser storage prefix, which stays homeBarSystem_. Changing it would sign everyone out on upgrade and strand their settings, and nobody ever sees it. Both places say why. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Everything on
staging, squashed intomain. This publisheslatestto the new image address.Tested on the real host before cutting.
The shape of it changed
Three containers became one. The API, the live updates, the drink photos and the pages are all served on a single port, and the database is brought up to date in-process on start. The one-shot init container that made Dockge report the stack as exited is gone.
The port is bound to
127.0.0.1, so the bar is only reachable through a proxy.Two real defects fixed
A SQL injection. The orders report built its date window by pasting
?days=straight into the query. Now validated as a whole number from 1 to 365 and passed as a parameter.The past orders page never loaded anything. It read whatever the menu had already fetched, so opening it from a bookmark, or refreshing it, showed an empty list however many drinks a guest had had.
Also along the way: the live connection was being torn down and reopened on every single order; a drink with no recipe crashed the recipe view; and logout wiped the whole origin's browser storage rather than this app's own keys.
Live updates
WebSockets replaced with a one-way stream. The browser reconnects on its own, which the old version had to hand-roll and gave up after three attempts, so a phone that slept stopped receiving orders until someone refreshed it. Guests are only told something is wrong if it stays down for 15 seconds.
The code
The server is TypeScript. The shapes the API sends live in one file both halves import, so changing one side without the other is a build error. Turning that on surfaced eight live defects, including flags typed as booleans that actually arrive as
0and1.Routes share a small set of pieces rather than repeating themselves, and no route needs a try/catch. The four oversized components were broken up. Both linters are clean at zero warnings, and
anyis an error in both halves.Tests
There were none. There are now 109: 70 for the server, 39 for the pages. The page tests run against a stand-in browser, so they need neither a server nor a real browser. Both are in CI, along with the linters, the typecheckers and a build for Intel and ARM.
The database
Migrations are recorded with a fingerprint of the file that made them, so an edit to one already applied stops the start rather than letting the database and the code drift apart. A migration that both creates a table and adds a column is now caught up a statement at a time instead of being written off whole.
Verified against a copy of the production database throughout.
Renamed to Barkeep
The image is now
ghcr.io/mtaanquist/barkeep. The old package keeps everything it already has, so nothing running breaks, but the compose file needs pointing at the new address to get updates.The browser storage prefix deliberately stays as it was, so nobody is signed out by the rename.