Skip to content

Conversation

@karm1000
Copy link
Member

@karm1000 karm1000 commented Oct 15, 2025

Traceback (most recent call last):
  File "/home/frappe/frappe-bench/env/lib/python3.11/site-packages/rq/worker.py", line 1428, in perform_job
    rv = job.perform()
         ^^^^^^^^^^^^^
  File "/home/frappe/frappe-bench/env/lib/python3.11/site-packages/rq/job.py", line 1278, in perform
    self._result = self._execute()
                   ^^^^^^^^^^^^^^^
  File "/home/frappe/frappe-bench/env/lib/python3.11/site-packages/rq/job.py", line 1315, in _execute
    result = self.func(*self.args, **self.kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/frappe/frappe-bench/apps/frappe/frappe/utils/background_jobs.py", line 225, in execute_job
    retval = method(**kwargs)
             ^^^^^^^^^^^^^^^^
  File "/home/frappe/frappe-bench/apps/india_compliance/india_compliance/gst_india/utils/__init__.py", line 1072, in create_notification
    notification.insert()
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 302, in insert
    self.check_permission("create")
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 228, in check_permission
    self._handle_permission_failure(permtype)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/model/document.py", line 247, in _handle_permission_failure
    check_doctype_permission(self.doctype, perm_type)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/permissions.py", line 861, in check_doctype_permission
    frappe.has_permission(doctype, ptype, throw=True, ignore_share_permissions=True)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/__init__.py", line 1065, in has_permission
    raise frappe.PermissionError
frappe.exceptions.PermissionError

Summary by CodeRabbit

  • Bug Fixes
    • Improved reliability of notification logging to ensure records are consistently created, reducing missed or incomplete entries in edge cases.
    • Enhanced stability of background notification creation, preventing failures that could previously occur in restricted scenarios and improving overall tracking accuracy.

@coderabbitai
Copy link

coderabbitai bot commented Oct 15, 2025

Walkthrough

Adjusted create_notification in india_compliance/gst_india/utils/init.py to insert Notification Log records with ignore_permissions=True, bypassing permission checks during persistence. No other logic, validation, or error handling was modified.

Changes

Cohort / File(s) Summary
Notification persistence change
india_compliance/gst_india/utils/__init__.py
Modified create_notification to call notification.insert(ignore_permissions=True), skipping permission checks at insert time; no other functional changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Caller
  participant U as utils.__init__.py (create_notification)
  participant DB as Database

  C->>U: create_notification(payload)
  U->>DB: insert(Notification Log, ignore_permissions=true)
  DB-->>U: inserted doc
  U-->>C: return result

  Note over U,DB: Change: insertion bypasses permission checks
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hopped through logs with gentle cheer,
A shortcut past the guards appeared—
Insert I shall, no keys to show,
A whisper to the database below.
Carrots saved, permissions stayed,
One tiny tweak, a smoother trade. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately and succinctly reflects the core change—bypassing permission checks during notification insertion—matching the actual modification in create_notification without introducing extraneous details.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 50820eb and dff68e6.

📒 Files selected for processing (1)
  • india_compliance/gst_india/utils/__init__.py (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Python Unit Tests
  • GitHub Check: Mergify Merge Protections
  • GitHub Check: Summary
🔇 Additional comments (1)
india_compliance/gst_india/utils/__init__.py (1)

1081-1081: Approve ignore_permissions=True on notification.insert
No background-job usages of create_notification detected; session user context unaffected.


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.

@codecov
Copy link

codecov bot commented Oct 15, 2025

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 69.60%. Comparing base (50820eb) to head (dff68e6).
⚠️ Report is 7 commits behind head on develop.

Files with missing lines Patch % Lines
india_compliance/gst_india/utils/__init__.py 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (85.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3746   +/-   ##
========================================
  Coverage    69.60%   69.60%           
========================================
  Files          182      182           
  Lines        17974    17974           
========================================
  Hits         12511    12511           
  Misses        5463     5463           
Files with missing lines Coverage Δ
india_compliance/gst_india/utils/__init__.py 73.48% <0.00%> (ø)

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mergify mergify bot merged commit c312d10 into resilient-tech:develop Oct 31, 2025
14 checks passed
mergify bot added a commit that referenced this pull request Oct 31, 2025
…tfix/pr-3746

fix: ignore permissions for notification insertion (backport #3746)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants