[utharakannan]

Recorded video

Summary

excel view bugs found on testing , fixed

Words used:
"never built" = the PDF describes it, but the app has no code for it.
"by design" = the app works this way on purpose; the PDF is out of date.

Local fix status (added 07 Oct 2026)
"Local" lines below say what happened to each bug in the code on the LOCAL site (http://localhost:8000).
The code is in apps/excel_view on branch fix/common-site-fixes. It is NOT committed, NOT pushed and NOT on
staging, so on jbcn-staging every bug still behaves as described, except where the staging scripts help.
FIXED = fixed in the code and tested in Chrome on the local site.
FIXED (small) = the feature already worked; a small change was made and tested.
PARTLY = part of it is fixed, part is not.
NOT FIXED = no code fix. The reason is given.
Details, code lines and test steps: "Bug Fix Code Map.txt".

1. Clear cache is not working
   Bug : clear cache does not work.
   Cause : the report gives no details, so the cause was not investigated.
   Local : NOT FIXED. No code change. The report gave no steps, so there is nothing to test or fix yet.

2. A bad Link value does not turn the cell red
   Bug : a Program that does not exist is accepted and saved.
   Cause : the grid checks the value only AFTER it accepts the edit. It flashes the cell red for half a second,
   but the edit is still saved. On Student Applicant the server then saved the wrong Program with no error.
   Note: on the School doctype a wrong Branch WAS refused by Frappe's own check, so the server does not
   always skip it. Why Program was saved is not confirmed.
   Local : FIXED. Real cause found in the code: Frappe's own link check skips a Link field that has "fetch from"
   fields (Program fills class_name), so a made-up Program was never reported. api.py now checks every
   Link value itself (_check_link_values) for every doctype. Tested: the value is refused and not stored,
   and the cell turns red with the message.

3. One bad value among several rows
   Bug : the bad row is saved, and no error shows on that row.
   Cause : the server did not report the bad value as an error. When it does report errors, the grid shows only
   the first one in a pop-up and does not mark the failed cell.
   Local : FIXED. Each row is saved on its own, a failed row is rolled back and reported with its cells, and the
   grid marks every failed cell red. Tested: of three rows only the bad one turned red, the others saved.

4. Undo / Redo does not work
   Bug : Ctrl+Z and Ctrl+Y do nothing, and the menu items are greyed out.
   Cause : Undo works in the grid, but about 1 second after Enter the grid auto-saves and reloads all rows. The
   reload erases the undo list, so Undo is available for only about 1.5 seconds.
   Local : FIXED. A new undo_manager.js keeps its own list by record name and field, so the reload no longer
   erases it. Tested: edit, wait for the reload, Ctrl+Z and Ctrl+Y both work.

5. A shared workbook shows others only what they may see
   Bug : User A saw an empty Saved Views list.
   Cause : not found. By the code, a view saved with "Share with everyone" should be listed. It could not be
   repeated, so it is NOT CONFIRMED.
   Local : NOT FIXED. No code change. The cause was not found and the empty list could not be repeated. To
   test it, share a view as one user and open Saved Views as another (see Bug Fix Code Map.txt).

6. Smart Lookup
   Bug : it only showed "Load data into at least 2 sheets".
   Cause : not a bug. Smart Lookup joins two sheets, and a new view has only one. A second sheet is needed.
   Local : NOT FIXED. Nothing to fix. It is not a bug. Add a second sheet (the + at the bottom left) to use it.

7. Flash Fill (Ctrl+E)
   Bug : nothing was filled, or the message "Could not detect a pattern" showed.
   Cause : not a bug in the feature. The first test sent a capital E and the app listens for a small e. Flash
   Fill only works when the examples you type can be built from a nearby column (first word, last word,
   initials, upper or lower case, and so on). Unrelated words give the "no pattern" message.
   Local : FIXED (small). Flash Fill already worked. Ctrl+E and Ctrl+D now also work with Caps Lock on. Tested:
   filled 98 cells with both Ctrl+E and Ctrl+e.

8. A view-only user sees no editable cells
   Bug : not tested at first (both logins could edit).
   Cause : tested later with a view-only login. No cell was editable, and a direct save was refused with a 403.
   Gap: the Bulk Add button still adds blank editable rows with no warning. The server refuses creating
   them (403), so nothing is stored; only the warning is missing.
   Local : NOT FIXED (partly fine). The server already refuses a view-only user's save (403), so nothing needed
   fixing there. The missing warning in Bulk Add is not fixed: no code was written for it.

9. Ctrl+Shift+C does not open Choose Columns
   Bug : nothing happens.
   Cause : never built. The app has no key listener for it.
   Local : FIXED. Ctrl+Shift+C and Alt+Shift+C now open Choose Columns. Tested in the test Chrome. Real Chrome may
   keep Ctrl+Shift+C for its developer tools; Alt+Shift+C is the safe key.

10. "No auto-save, press Ctrl+S" is wrong
    Bug : the value was saved about 1 second after Enter, with no Ctrl+S.
    Cause : by design. A timer saves every edit about 1 second after it is made, and the grid has no Ctrl+S
    code. The PDF describes an older design.
    Local : FIXED. A site-wide "Manual Save (Ctrl+S)" switch, ON by default. Edits wait until Ctrl+S, a cell still
    being typed is saved too, leaving the page saves the waiting edits, and closing the tab asks first.
    Tested: 0 saves in 5 seconds, then Ctrl+S saved them in one call. Untick the box at
    /app/excel-view-settings to get automatic saving back. It changes how every user saves.

11. Read-only fields are not blocked by the server
    Bug : a direct save to a read-only field (workspace_email) was accepted and changed the value.
    Cause : only the grid stops edits to read-only fields. The server function (bulk_set_value) saves any field
    it is given and does not check the read-only flag.
    Local : FIXED. The server now refuses read-only, system (owner, docstatus...), layout, table and unknown
    fields. Tested on the server, and the same check passed on 324 doctypes that have records locally.

12. Funnel icon (Toggle Filters) shows no filter bar
    Bug : the funnel turns on, but no bar appears.
    Cause : the app's own style rule hides it: .ev-page .page-form { display: none !important }. The button
    cannot override it.
    Local : FIXED. The stylesheet now shows the filter bar when the funnel is on. Tested: the bar appeared (126 px)
    and hid on the second click.

13. "Clear Filters" button is missing
    Bug : no such button exists.
    Cause : never built.
    Local : FIXED. A "Clear Filters" button now sits next to the funnel. Tested: 2 rows back to 100 rows.

14. Right-click header "Filter by this value" is missing
    Bug : no such menu item.
    Cause : never built. The right-click menu is a fixed list without it.
    Local : FIXED. "Filter by this value" is now in the cell right-click menu. Tested: 100 rows reduced to 2.

15. Ctrl+Shift+F does not open a filter
    Bug : nothing happens.
    Cause : never built.
    Local : NOT FIXED. No code was written. Use the funnel or "Filter by this value" instead, and correct the
    PDF.

16. Ctrl+Shift+W does not open Workbook
    Bug : nothing happens.
    Cause : never built. Also, Chrome itself closes the window with this key, so a page cannot use it (known
    browser behaviour, not tested here).
    Local : NOT FIXED. Cannot be fixed in a page, because Chrome closes the window with this key. Correct the PDF.

17. "Save as New Workbook" button is missing
    Bug : no such button.
    Cause : never built. The nearest item is "Save As..." in the More options menu, and that menu is hidden
    (see case 19).
    Local : PARTLY. "More options > Save As..." - not showing previously, now it is working

18. Formulas (=SUM, =FRAPPE_COUNT, =FRAPPE_SUM) show empty cells
    Bug : the cells looked empty after typing a formula.
    Cause : the formulas work (FRAPPE_COUNT, FRAPPE_SUM, FRAPPE_GET and FRAPPE_AVG matched the database). A typed
    formula such as =SUM(1,2,3) is calculated, but the cell is not redrawn, so it looks empty until the
    grid redraws.
    Local : FIXED. The grid is redrawn at once after a formula is typed. Tested: =SUM(1,2,3) showed 6 within 0.5
    seconds.

19. "More options" menu (Save As...) cannot be seen
    Bug : the arrow opens a menu, but it is not visible and a mouse click misses it.
    Cause : the menu hangs below the ribbon, which is only about 34 px high and cuts off anything that sticks
    out. The click handler itself works.
    Local : FIXED. The menu is now drawn on top of the page under the arrow. Tested: it is visible and a mouse
    click on "Save As..." opened the Save View dialog.

[sankarsubramaniankvs]

Thanks for the fixes here. I’m holding this from merge for a few issues/risks that need to be addressed first.

Blockers:

1. Read-only field protection still appears bypassable for fetched fields

In excel_view/api.py, _check_writable_fields() rejects read-only fields only when they do not have fetch_from:

if df.read_only and not df.fetch_from:

That means a read-only field with fetch_from can still pass through bulk_set_value. Since this is a direct server API path, I don’t think fetched/derived read-only fields should be writable unless there is a very explicit allowlist with tests.

Suggested fix: reject all df.read_only fields here by default, and add a regression test for read-only + fetch_from fields.

2. No checks reported for a very large behavior change

GitHub reports no CI/checks on this branch. This PR changes server save behavior, permissions/validation, undo/redo, filtering/formula behavior, toolbar/context menu UI, settings schema, and compiled assets. Please add/run targeted validation before merge.

At minimum, I’d want coverage/manual evidence for:

• bulk_set_value refusing read-only fields, including fetch_from
• invalid Link values being rejected
• partial-row save rollback behavior
• permission-denied save paths
• manual-save/auto-save behavior across reload/navigation

3. Manual-save default changes existing behavior globally

The new Manual Save setting appears to default ON / changes the save model for all users. That is a product-level behavior change, not just a bugfix. Please either default to preserving existing auto-save behavior, or get explicit product approval and document the migration/release-note impact, including how unsaved edits are protected from reload/navigation/session expiry.

Other concerns:

• This PR is very broad for a “bugfix” and mixes security/server fixes, UX behavior changes, DocType/settings changes, generated JS bundle changes, and large SCSS changes. Splitting it would make review and rollback much safer.
• Please confirm the generated bundle/SCSS changes are intentionally rebuilt from source and not unrelated drift.

Not merging per request; please address the blockers above before this lands.

[sankarsubramaniankvs]

Re-reviewing latest commit 679c12d9b62385c3821beb60bbb79f4d44b17f05.

The prior code blockers look addressed:

• _check_writable_fields() now rejects all df.read_only fields, including fetch_from, and also rejects fieldtype == "Read Only".
• Added regression coverage for read-only + fetch_from, Read Only + fetch_from, invalid Link values, partial-row rollback, permission-denied paths, and manual-save setting defaults in excel_view/tests/test_bulk_save.py.
• Excel View Settings.manual_save now defaults to 0, and _manual_save_enabled() returns 0 when unset, preserving existing auto-save behavior unless explicitly enabled.
• CI workflow was added at .github/workflows/ci.yml.

I’m not approving yet because the new GitHub check (Server tests) is still pending/in progress on the latest commit. Once that completes green, this should be approvable from my side unless the run exposes failures.