Sitelet https://github.com/opensourcepos/opensourcepos/pull/4633
Skip to content

fix: refresh expenses table when date range or filters change - #4633

Open
richardmilles wants to merge 4 commits into
opensourcepos:masterfrom
richardmilles:fix-4596-expenses-daterangepicker
Open

fix: refresh expenses table when date range or filters change#4633
richardmilles wants to merge 4 commits into
opensourcepos:masterfrom
richardmilles:fix-4596-expenses-daterangepicker

Conversation

@richardmilles

@richardmilles richardmilles commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Restores the missing daterangepicker apply and filter change handlers on the Expenses page so selecting a date range reloads the table with the correct start_date and end_date.

Without these handlers the UI range updated but /expenses/search kept today's date for both parameters, so historical expenses never appeared.

Fixes #4596

Summary by CodeRabbit

  • Bug Fixes
    • Expense tables now refresh automatically when filters are closed or a date range is applied.
    • Existing date-range picker behavior remains unchanged.

Restore the daterangepicker apply and filter change handlers so /expenses/search receives the selected start_date and end_date instead of today's date for both.

Fixes opensourcepos#4596
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ec4b7215-afca-47c0-8b05-f8dc43b0df21

📥 Commits

Reviewing files that changed from the base of the PR and between aa96ad0 and 15eb899.

📒 Files selected for processing (1)
  • app/Views/expenses/manage.php

📝 Walkthrough

Walkthrough

The expenses view now refreshes the expense table when the filter dropdown closes and when a date range is applied. The existing date-range picker initialization remains unchanged.

Changes

Expense table refresh

Layer / File(s) Summary
Wire expense filter refresh events
app/Views/expenses/manage.php
The expense table refreshes after filter dropdown closure and date-range application.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the expense table refresh changes for date-range and filter updates.
Linked Issues check ✅ Passed The change restores expense table handlers for date-range and filter changes, addressing the selected start_date and end_date issue in [#4596].
Out of Scope Changes check ✅ Passed The changes are limited to expense table refresh behavior and align with the requirements in [#4596].
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jekkos

jekkos commented Aug 7, 2026

Copy link
Copy Markdown
Member

@CodeRabbit was this pattern missing in expenses view specifcally?

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== PR diff ==='
git diff HEAD^ HEAD -- app/Views/expenses/manage.php

echo
echo '=== Expenses view outline ==='
ast-grep outline app/Views/expenses/manage.php --items all || true

echo
echo '=== Date-range and select-close handler patterns in views ==='
rg -n -C 4 --glob '*.php' \
  '(daterangepicker|apply\.daterangepicker|close\.bs\.select|start_date|end_date)' \
  app/Views

Length of output: 26191


@jekkos Yes. Before this PR, app/Views/expenses/manage.php did not refresh the table after either event:

  • hidden.bs.select for the filter dropdown.
  • apply.daterangepicker for the date-range picker.

app/Views/sales/manage.php already has the same refresh pattern. The Expenses view already included start_date and end_date in queryParams; it only failed to refresh after the user changed the range.

This pattern is not present in every date-range view. For example, app/Views/cashups/manage.php also has no equivalent refresh handlers.

You are interacting with an AI system.

@jekkos

jekkos commented Aug 7, 2026

Copy link
Copy Markdown
Member

@CodeRabbit is this bug also present in other views?

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Date-range picker candidates ==='
rg -n -C 5 --glob '*.php' \
  '(daterangepicker|apply\.daterangepicker|start_date|end_date|refreshOptions|bootstrapTable)' \
  app/Views

echo
echo '=== View structure for files that initialize daterangepicker ==='
while IFS= read -r file; do
  echo
  echo "--- $file ---"
  ast-grep outline "$file" --items all || true
done < <(rg -l --glob '*.php' 'daterangepicker\s*\(' app/Views)

Length of output: 35525


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Cashups view script and shared partial inclusions ==='
sed -n '15,50p' app/Views/cashups/manage.php
rg -n -C 3 "table_filter_persistence|apply\.daterangepicker|hidden\.bs\.select|table_support\.refresh" \
  app/Views/cashups/manage.php

echo
echo '=== Items view script and shared partial inclusions ==='
sed -n '15,85p' app/Views/items/manage.php
rg -n -C 3 "table_filter_persistence|apply\.daterangepicker|hidden\.bs\.select|table_support\.refresh" \
  app/Views/items/manage.php

echo
echo '=== All manage views that initialize the shared date-range picker ==='
rg -n -C 3 --glob 'manage.php' \
  "view\('partial/daterangepicker'\)|apply\.daterangepicker|hidden\.bs\.select|table_support\.refresh" \
  app/Views

echo
echo '=== Uses of the table-filter persistence partial ==='
rg -n -C 3 "view\('partial/table_filter_persistence'\)" app/Views

Length of output: 11031


@jekkos Yes. The same date-range refresh defect exists in these views:

  • app/Views/cashups/manage.php
  • app/Views/items/manage.php

Both views update the shared start_date and end_date variables. Both also send those values in queryParams. Neither view calls table_support.refresh() on apply.daterangepicker.

partial/table_filter_persistence only updates the URL. It does not reload the table.

cashups/manage.php also has no hidden.bs.select handler for its filter dropdown. items/manage.php has no local refresh handler for its filters or stock-location selector.

The report input views do not have this defect. They use the selected dates when the user clicks Generate Report, instead of reloading a main table.

You are interacting with an AI system.

@jekkos

jekkos commented Aug 7, 2026

Copy link
Copy Markdown
Member

@CodeRabbit I will reject this change ideally we also use filter persistence once a range is selected, we change the URL

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@jekkos Acknowledged. The correct behavior is to persist the selected date range and update the URL when the user applies the range. A table refresh alone is not sufficient.

🐇︵︵


✏️ Learnings added
Learnt from: jekkos
URL: https://github.com/opensourcepos/opensourcepos/pull/4633

Timestamp: 2026-08-07T18:21:19.165Z
Learning: In `app/Views/*/manage.php` date-range filters, applying a date range must use the table-filter persistence behavior to update the URL with the selected range. A local `table_support.refresh()` handler alone is incomplete.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

@jekkos

jekkos commented Aug 7, 2026

Copy link
Copy Markdown
Member

@CodeRabbit can you male these changes?

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Candidate files ==='
fd -t f -p 'manage.php' app/Views/expenses app/Views/sales app/Views || true
fd -t f 'table_filter_persistence.php' app/Views || true
fd -t f 'daterangepicker.php' app/Views || true

echo
echo '=== Expenses view outline ==='
ast-grep outline app/Views/expenses/manage.php --items all

echo
echo '=== Expenses date-range and table handlers ==='
rg -n -C 8 \
  '(queryParams|daterangepicker|apply\.daterangepicker|hidden\.bs\.select|table_support\.refresh|table_filter_persistence|start_date|end_date)' \
  app/Views/expenses/manage.php

echo
echo '=== Shared persistence partial ==='
persistence_file="$(fd -t f 'table_filter_persistence.php' app/Views | head -n 1)"
if [ -n "$persistence_file" ]; then
  cat -n "$persistence_file"
fi

echo
echo '=== Shared date-range partial ==='
daterange_file="$(fd -t f 'daterangepicker.php' app/Views | head -n 1)"
if [ -n "$daterange_file" ]; then
  cat -n "$daterange_file"
fi

echo
echo '=== Comparable manage-view event handlers ==='
rg -n -C 8 --glob 'manage.php' \
  '(apply\.daterangepicker|hidden\.bs\.select|table_filter_persistence|table_support\.refresh)' \
  app/Views

Length of output: 32673


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== All persistence-partial consumers ==='
rg -n -C 12 "view\('partial/table_filter_persistence'" app

echo
echo '=== Consumer date and filter handlers ==='
while IFS= read -r file; do
  echo
  echo "--- $file ---"
  rg -n -C 8 \
    '(hidden\.bs\.select|apply\.daterangepicker|table_support\.refresh|daterangepicker|filters)' \
    "$file" || true
done < <(rg -l "view\('partial/table_filter_persistence'" app)

echo
echo '=== Candidate view and JavaScript tests ==='
fd -t f . tests app | rg '(Expenses|expenses|table_filter_persistence|daterangepicker|manage).*([Tt]est|test|spec)\.(php|js)$' || true

echo
echo '=== References to custom jQuery events for table persistence ==='
rg -n -C 4 \
  '(trigger\s*\(|\.on\s*\().*(filter|persist|url|table_support)' \
  app tests || true

Length of output: 8625


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== Persistence-partial consumers ==='
rg -n -C 10 "view\('partial/table_filter_persistence'" app

echo
echo '=== Items handlers ==='
rg -n -C 10 \
  '(hidden\.bs\.select|apply\.daterangepicker|table_support\.refresh|daterangepicker|filters|stock_location)' \
  app/Views/items/manage.php || true

echo
echo '=== Cashups handlers ==='
rg -n -C 10 \
  '(hidden\.bs\.select|apply\.daterangepicker|table_support\.refresh|daterangepicker|filters)' \
  app/Views/cashups/manage.php || true

echo
echo '=== Expenses handlers ==='
rg -n -C 10 \
  '(hidden\.bs\.select|apply\.daterangepicker|table_support\.refresh|daterangepicker|filters)' \
  app/Views/expenses/manage.php || true

echo
echo '=== Sales handlers ==='
rg -n -C 10 \
  '(hidden\.bs\.select|apply\.daterangepicker|table_support\.refresh|daterangepicker|filters)' \
  app/Views/sales/manage.php || true

echo
echo '=== Candidate tests ==='
fd -t f . tests app 2>/dev/null \
  | rg -i '(expenses|table_filter_persistence|daterangepicker|manage).*(test|spec).*?\.(php|js)$' \
  || true

echo
echo '=== Existing custom event conventions ==='
rg -n -C 4 \
  '\.(trigger|triggerHandler|on)\s*\(\s*["'\''][^"'\'']+(filter|persist|url)[^"'\'']*["'\'']' \
  app tests 2>/dev/null \
  || true

Length of output: 22752


@jekkos The shared persistence partial already updates the URL. The handler order is the problem.

I will move the existing partial/table_filter_persistence include above the Expenses script. The persistence handler will then run before table_support.refresh() for both date-range and filter changes.

  • Modify only app/Views/expenses/manage.php.
  • Move <?= view('partial/table_filter_persistence') ?> from below the Expenses script to above it.
  • Keep the existing apply.daterangepicker refresh handler.
  • Do not duplicate URL construction or modify the shared partial.

  • ❌ Failed to create commit - (🔄 Check again to retry)

You are interacting with an AI system.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

❌ Failed to create commit: AGENT_CHAT: Failed to create commit

1 similar comment
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

❌ Failed to create commit: AGENT_CHAT: Failed to create commit

@jekkos

jekkos commented Aug 9, 2026

Copy link
Copy Markdown
Member

@CodeRabbit what permission needs to be granted to allow you to push commits

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@jekkos I will check the current CodeRabbit configuration and the required GitHub permission before I give you the exact setting.

[coderabbit_configuration]
Check the current CodeRabbit configuration and schema. Identify the GitHub App permission required for CodeRabbit to create and push commits to a pull request branch, and report any configuration or branch-protection prerequisites.
[/coderabbit_configuration]

You are interacting with an AI system.


<script type="text/javascript">
$(document).ready(function() {
// When any filter is clicked and the dropdown window is closed

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove all the comments from here. They aren't needed. Also from lines 22, 31, et al.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Expenses date-range filter sends the end date as both start_date and end_date

3 participants