Skip to content

fix(accounting): transaction status improve#7737

Merged
munkhsaikhan merged 13 commits into
mainfrom
improve
May 20, 2026
Merged

fix(accounting): transaction status improve#7737
munkhsaikhan merged 13 commits into
mainfrom
improve

Conversation

@munkhsaikhan
Copy link
Copy Markdown
Collaborator

@munkhsaikhan munkhsaikhan commented May 20, 2026

Summary by Sourcery

Adjust transaction status classification and visibility rules across backend and frontend.

Bug Fixes:

  • Ensure conversation-status transactions are only visible to their creator or mentioned users instead of all users.

Enhancements:

  • Expand ACTIVE transaction statuses to include cancelled transactions for backend filtering.
  • Introduce a CONVERSATION status group in the backend for draft, mentioned, approved, rejected, returned, and planned transactions.
  • Update frontend transaction status group labels to DRAFTED and PUBLISHED and hide label for the PLAN-only group when empty.
  • Render status group labels in the transaction form dropdown only when defined.

Summary by CodeRabbit

Release Notes

  • User Interface Updates
    • Updated transaction status labels to English ("DRAFTED", "PUBLISHED").
    • Reorganized transaction status grouping structure for better clarity and organization.
    • Status groups now display with clear visual labeling in transaction forms.

Review Change Stack

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 20, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Updates transaction status categorization and visibility rules: backend now classifies more statuses as ACTIVE and introduces a CONVERSATION group, while frontend status group labels and dropdown rendering are aligned with the new grouping and visibility behavior based on user association with a transaction.

Flow diagram for updated transaction visibility filter logic

flowchart TD
  A[generateFilter called
  with user and params] --> B[Check if user exists]
  B -->|yes| C[Initialize andFilter]
  C --> D[Add $or filter]
  D --> E[Condition 1:
  status in TR_STATUSES.ACTIVE]
  D --> F[Condition 2:
  status in TR_STATUSES.CONVERSATION
  AND user is associated]
  F --> G[Subcondition:
  createdBy equals user._id]
  F --> H[Subcondition:
  mentionOwnerId equals user._id]
  F --> I[Subcondition:
  mentionUserIds contains user._id]
  E --> J[Return filter]
  G --> J
  H --> J
  I --> J
Loading

File-Level Changes

Change Details Files
Adjust transaction visibility filter so conversation-status transactions are only visible when associated with the current user.
  • Replaced separate DRAFT, mentionOwnerId, and mentionUserIds visibility conditions with a single CONVERSATION status branch that also checks createdBy, mentionOwnerId, or mentionUserIds against the current user.
  • Continues to allow visibility for ACTIVE statuses without user-specific constraints.
backend/plugins/accounting_api/src/modules/accounting/graphql/resolvers/queries/transactionsCommon.ts
Refine transaction status groups and labels in the frontend to reflect new ACTIVE/CONVERSATION backend categorizations and hide empty group labels.
  • Renamed the draft-related group label to DRAFTED and the in-progress group label to PUBLISHED.
  • Cleared the label text for the PLAN group to avoid displaying a separate heading for this single status.
  • Updated the Select dropdown to only render a group label when one is defined, preventing empty labels from showing.
frontend/plugins/accounting_ui/src/modules/transactions/types/constants.tsx
frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/TransactionsGroupForm.tsx
Extend backend transaction status constants to match new grouping semantics for ACTIVE and CONVERSATION statuses.
  • Expanded ACTIVE status list to include the cancelled state so it is treated as active for filtering purposes.
  • Introduced a new CONVERSATION status list covering draft, mentioned, approved, rejected, returned, and plan states for use in filters and grouping.
backend/plugins/accounting_api/src/modules/accounting/@types/constants.ts

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

📝 Walkthrough

Walkthrough

Transaction status handling is reorganized across backend and frontend. Backend constants introduce a new CONVERSATION grouping for conversation-level statuses while extending ACTIVE to include cancelled. Filtering logic updates to conditionally apply conversation statuses based on user relationships. Frontend labels switch from Mongolian to English display names, and UI rendering displays group headings in the status dropdown.

Changes

Transaction Status Grouping and Display

Layer / File(s) Summary
Backend status grouping constants
backend/plugins/accounting_api/src/modules/accounting/@types/constants.ts
TR_STATUSES.ACTIVE now includes canelled alongside progress, assigned, confirmed, and complete. New TR_STATUSES.CONVERSATION group separates conversation-level statuses (draft, mentioned, approved, rejected, returned, plan) from other categories.
Backend filtering logic with conversation grouping
backend/plugins/accounting_api/src/modules/accounting/graphql/resolvers/queries/transactionsCommon.ts
Default generateFilter branch replaces TR_STATUSES.DRAFT condition with TR_STATUSES.CONVERSATION, conditionally applied only when the transaction matches user-related fields (createdBy, mentionOwnerId, or mentionUserIds).
Frontend status group display labels
frontend/plugins/accounting_ui/src/modules/transactions/types/constants.tsx
TR_STATUS_GROUPS labels are updated from Mongolian to English: first group becomes DRAFTED, second becomes PUBLISHED, third (plan) becomes an empty string.
Frontend UI grouping and label rendering
frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/TransactionsGroupForm.tsx
Status <Select> component now conditionally renders <Select.Label> for each group when group.label is present, displaying group headings above grouped options.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • erxes/erxes#7658: Updates to transaction status definitions, the CONVERSATION grouping logic, and the corresponding mention/status filtering in transactionsCommon.ts overlap directly with this PR's backend and filtering changes.

Poem

🐰 Statuses grouped with care so true,
Conversations apart from the rest of the crew,
Mongolian words to English we shift,
With labels that guide—a delightful gift!
From backend to frontend, the grouping flows free,

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'transaction status improve' is vague and overly generic. It uses a non-descriptive term ('improve') that doesn't convey meaningful information about what specific changes were made to transaction status handling. Make the title more specific by describing the actual changes, such as 'refactor(accounting): add conversation status group and update transaction filtering' or 'refactor(accounting): reorganize transaction statuses with new conversation group'.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch improve

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 and usage tips.

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • In generateFilter, status: TR_STATUSES.CONVERSATION is now an array but is used like a scalar; this should likely be status: { $in: TR_STATUSES.CONVERSATION } to match how ACTIVE is handled.
  • The new TR_STATUSES.CONVERSATION array includes a status key rejeced, which looks like a typo compared to the existing rejected status naming and may cause mismatches when querying or filtering.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `generateFilter`, `status: TR_STATUSES.CONVERSATION` is now an array but is used like a scalar; this should likely be `status: { $in: TR_STATUSES.CONVERSATION }` to match how `ACTIVE` is handled.
- The new `TR_STATUSES.CONVERSATION` array includes a status key `rejeced`, which looks like a typo compared to the existing `rejected` status naming and may cause mismatches when querying or filtering.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@github-actions
Copy link
Copy Markdown

🌗 Pull Request Overview

This PR improves the transaction status handling in the accounting module by adding 'cancelled' to the ACTIVE status filter and introducing a new CONVERSATION status category. It also refactors the transaction filtering logic and updates frontend UI labels from Mongolian to English.

Reviewed Changes
Kimi performed full review on 4 changed files and found 4 issues.

Show a summary per file
File Description
backend/plugins/accounting_api/src/modules/accounting/@types/constants.ts Added 'canelled' to ACTIVE array and new CONVERSATION status array
backend/plugins/accounting_api/src/modules/accounting/graphql/resolvers/queries/transactionsCommon.ts Refactored filter logic to use CONVERSATION status with nested OR conditions
frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/TransactionsGroupForm.tsx Added Select.Label rendering for group labels in status dropdown
frontend/plugins/accounting_ui/src/modules/transactions/types/constants.tsx Changed status group labels from Mongolian to English strings

📋 Review Findings

📄 backend/plugins/accounting_api/src/modules/accounting/@types/constants.ts

🔴 CRITICAL typo: Spelling errors in status values

Lines 81, 84, 85

Multiple spelling errors in status strings: 'canelled' should be 'cancelled' (appears twice), 'rejeced' should be 'rejected'. These typos will cause matching failures when filtering transactions since the actual database values likely use correct spellings.

💡 Suggested fix:

Current code:

'canelled',  // line 81
// ...
'canelled',  // line 84
'rejeced',   // line 85

Improved code:

'cancelled',  // fix spelling - appears at lines 81 and 84
'rejected',   // fix spelling - line 85

📄 backend/plugins/accounting_api/src/modules/accounting/graphql/resolvers/queries/transactionsCommon.ts

🟡 MEDIUM logic: Filter behavior change may expose unintended data

Lines 349-357

The refactored filter logic groups draft status with CONVERSATION statuses. Previously, only drafts created by the user were shown (createdBy: user._id). Now, users will see ALL transactions with CONVERSATION statuses (draft, mentioned, approved, rejected, returned, plan) where they are mentioned - including drafts created by others. This significantly expands data visibility.

💡 Suggested fix:

If the intent was to maintain the original behavior (user only sees their own drafts), restrict draft visibility:

{
  $or: [
    { status: { $in: TR_STATUSES.ACTIVE } },
    {
      $and: [
        { status: { $in: TR_STATUSES.CONVERSATION } },
        {
          $or: [
            { $and: [{ status: TR_STATUSES.DRAFT }, { createdBy: user._id }] },
            { mentionOwnerId: user._id },
            { mentionUserIds: { $in: [user._id] } },
          ]
        }
      ]
    }
  ],
}

📄 frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/TransactionsGroupForm.tsx

🔵 LOW code quality: Conditional rendering logic

Line 134

The condition group.label && prevents rendering when label is an empty string. This is acceptable but could be more explicit. Consider whether an empty string group should show a visual separator or be truly label-less.

Current behavior is acceptable - no change required unless design requires different handling for unlabeled groups.


📄 frontend/plugins/accounting_ui/src/modules/transactions/types/constants.tsx

🟡 MEDIUM i18n: Hardcoded English labels instead of translation keys

Lines 154, 164

The labels were changed from Mongolian to hardcoded English strings. This breaks i18n best practices and will prevent future localization. These should use translation keys.

💡 Suggested fix:

export const TR_STATUS_GROUPS = [
  {
    label: i18n.t('transactions.statusGroups.drafted'),  // or translation key
    values: [
      TR_STATUSES.DRAFT,
      TR_STATUSES.MENTIONED,
      TR_STATUSES.APPROVED,
      TR_STATUSES.REJECTED,
      TR_STATUSES.RETURNED,
    ],
  },
  {
    label: i18n.t('transactions.statusGroups.published'),
    values: [
      TR_STATUSES.PROGRESS,
      TR_STATUSES.ASSIGNED,
      TR_STATUSES.CONFIRMED,
      TR_STATUSES.CANCELLED,
      TR_STATUSES.COMPLETE,
    ],
  },
  {
    label: '',  // Consider null or explicit 'none' key
    values: [TR_STATUSES.PLAN],
  },
];


Powered by Kimi | Model: kimi-k2.5

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@backend/plugins/accounting_api/src/modules/accounting/graphql/resolvers/queries/transactionsCommon.ts`:
- Around line 349-355: The query is comparing status to an array constant
(TR_STATUSES.CONVERSATION) directly which never matches; update the filter to
use MongoDB membership operator by replacing the plain comparison with a $in
check (i.e., change the condition for status in the query object to use { $in:
TR_STATUSES.CONVERSATION }) so the status field is matched against any value in
the TR_STATUSES.CONVERSATION array; locate this in the transactions query block
in transactionsCommon.ts where status is currently set to
TR_STATUSES.CONVERSATION.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ab2578d9-c139-429b-9917-57155f457ce0

📥 Commits

Reviewing files that changed from the base of the PR and between c9000b5 and fbf557b.

📒 Files selected for processing (4)
  • backend/plugins/accounting_api/src/modules/accounting/@types/constants.ts
  • backend/plugins/accounting_api/src/modules/accounting/graphql/resolvers/queries/transactionsCommon.ts
  • frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/TransactionsGroupForm.tsx
  • frontend/plugins/accounting_ui/src/modules/transactions/types/constants.tsx

Comment on lines +349 to +355
{
status: TR_STATUSES.CONVERSATION, $or: [
{ createdBy: user._id },
{ mentionOwnerId: user._id },
{ mentionUserIds: { $in: [user._id] } },
]
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# First, find and examine the file in question
cat -n backend/plugins/accounting_api/src/modules/accounting/graphql/resolvers/queries/transactionsCommon.ts | sed -n '340,365p'

Repository: erxes/erxes

Length of output: 806


🏁 Script executed:

# Search for TR_STATUSES definition
rg "TR_STATUSES" backend/plugins/accounting_api/src --type ts -A 2 -B 2 | head -100

Repository: erxes/erxes

Length of output: 9658


🏁 Script executed:

# Check for any constant files that might define TR_STATUSES
fd -e ts -e js backend/plugins/accounting_api/src | xargs rg "CONVERSATION" | grep -E "(const|export)" | head -20

Repository: erxes/erxes

Length of output: 3532


🏁 Script executed:

# Find the constants file
find backend/plugins/accounting_api/src -name "constants*" -type f

Repository: erxes/erxes

Length of output: 204


🏁 Script executed:

# Search for TR_STATUSES definition
rg "export.*TR_STATUSES" backend/plugins/accounting_api/src --type ts -A 30

Repository: erxes/erxes

Length of output: 6094


Use $in for conversation status matching.

Line 350 compares status to an array of values directly, which won't match any documents. Since TR_STATUSES.CONVERSATION is an array (['draft', 'mentioned', 'approved', 'rejeced', 'returned', 'plan']), it requires the $in operator for membership filtering, as already used correctly on line 348 with TR_STATUSES.ACTIVE.

Proposed fix
         {
-          status: TR_STATUSES.CONVERSATION, $or: [
+          status: { $in: TR_STATUSES.CONVERSATION }, $or: [
             { createdBy: user._id },
             { mentionOwnerId: user._id },
             { mentionUserIds: { $in: [user._id] } },
           ]
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{
status: TR_STATUSES.CONVERSATION, $or: [
{ createdBy: user._id },
{ mentionOwnerId: user._id },
{ mentionUserIds: { $in: [user._id] } },
]
}
{
status: { $in: TR_STATUSES.CONVERSATION }, $or: [
{ createdBy: user._id },
{ mentionOwnerId: user._id },
{ mentionUserIds: { $in: [user._id] } },
]
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@backend/plugins/accounting_api/src/modules/accounting/graphql/resolvers/queries/transactionsCommon.ts`
around lines 349 - 355, The query is comparing status to an array constant
(TR_STATUSES.CONVERSATION) directly which never matches; update the filter to
use MongoDB membership operator by replacing the plain comparison with a $in
check (i.e., change the condition for status in the query object to use { $in:
TR_STATUSES.CONVERSATION }) so the status field is matched against any value in
the TR_STATUSES.CONVERSATION array; locate this in the transactions query block
in transactionsCommon.ts where status is currently set to
TR_STATUSES.CONVERSATION.

@sonarqubecloud
Copy link
Copy Markdown

@munkhsaikhan munkhsaikhan merged commit af56792 into main May 20, 2026
16 of 18 checks passed
@munkhsaikhan munkhsaikhan deleted the improve branch May 20, 2026 07:58
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.

1 participant