fix: Fixed SLA resolution by calculation

[aasif-patel]

Fix: SLA resolution by date time calculation with hold time
This PR addresses an issue where the Resolution Time calculation incorrectly included hold time, leading to a total resolution duration that could exceed the defined SLA working hours or days.

The logic has been updated to account for hold time before calculating the Resolution By datetime. This ensures that the final Resolution By datetime remains accurate and in line with SLA settings.

SLA Configurations

Before

After

[coderabbitai]

📝 Walkthrough

Walkthrough

The change modifies the set_resolution_by method in the service level agreement doctype to adjust how hold time influences resolution deadline calculations. When total_hold_time is present and positive, the code now pre-adjusts the start_date_time by adding total_hold_time before computing the initial resolution deadline, then further adds total_hold_time to the calculated deadline. This results in total_hold_time being applied twice in the deadline computation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Single-file change with localized scope to one method, but the logic modification requires careful verification
• The dual application of total_hold_time (both to the start time reference and the final deadline) is unusual and warrants scrutiny to confirm intentionality and correctness
• Consider reviewing:
  • Whether the double-accounting of total_hold_time is intentional or a logic error
  • The broader context of SLA resolution deadline calculations and hold time semantics
  • Related test cases to verify expected behavior aligns with actual outcomes
  • Any existing documentation or requirements around hold time accounting in deadline calculations

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly relates to the main change: fixing SLA resolution by calculation to properly account for hold time in the resolution deadline computation.
Description check	✅ Passed	The description is directly related to the changeset, explaining the fix for SLA resolution time calculation with hold time and providing before/after visual evidence of the correction.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ea4e21 and abcfe8a.

📒 Files selected for processing (1)

• erpnext/support/doctype/service_level_agreement/service_level_agreement.py (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: aasif-patel
Repo: frappe/erpnext PR: 49414
File: erpnext/support/doctype/service_level_agreement/service_level_agreement.py:885-889
Timestamp: 2025-09-02T07:41:50.705Z
Learning: In the set_resolution_by function in service_level_agreement.py, the user prefers to add total_hold_time to start_date_time before calling get_expected_time_for rather than adding it to the result afterward, as this provides a more logical flow for SLA calculations.

📚 Learning: 2025-09-02T07:41:50.705Z

Learnt from: aasif-patel
Repo: frappe/erpnext PR: 49414
File: erpnext/support/doctype/service_level_agreement/service_level_agreement.py:885-889
Timestamp: 2025-09-02T07:41:50.705Z
Learning: In the set_resolution_by function in service_level_agreement.py, the user prefers to add total_hold_time to start_date_time before calling get_expected_time_for rather than adding it to the result afterward, as this provides a more logical flow for SLA calculations.

Applied to files:

• erpnext/support/doctype/service_level_agreement/service_level_agreement.py

⏰ 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). (1)

• GitHub Check: Summary

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}