Skip to content

Refactor duplicate error handling into fetchErrorMessage helper#7644

Merged
DawoudIO merged 3 commits intomasterfrom
copilot/sub-pr-7642
Nov 26, 2025
Merged

Refactor duplicate error handling into fetchErrorMessage helper#7644
DawoudIO merged 3 commits intomasterfrom
copilot/sub-pr-7642

Conversation

Copy link
Contributor

Copilot AI commented Nov 26, 2025

What Changed

Extracts duplicate error handling logic for 404/500 status codes in VerifyThenLoadAPIContent into a reusable fetchErrorMessage helper function.

Addresses review feedback from #7642.

Type

  • ✨ Feature
  • 🐛 Bug fix
  • ♻️ Refactor
  • 🏗️ Build/Infrastructure
  • 🔒 Security

Testing

No functional change. The helper function encapsulates the same AJAX call pattern previously duplicated in both 404 and 500 handlers.

Before (repeated in both handlers):

404: function () {
    try {
        $.ajax({
            method: "GET",
            url: url,
            async: false,
            dataType: "json",
            success: function (data) {
                var msg = data && data.message ? data.message : error;
                window.CRM.DisplayErrorMessage(url, { message: msg });
            },
            error: function () {
                window.CRM.DisplayErrorMessage(url, { message: error });
            },
        });
    } catch (e) {
        window.CRM.DisplayErrorMessage(url, { message: error });
    }
}

After:

404: function () {
    fetchErrorMessage(url, error, function (msg) {
        window.CRM.DisplayErrorMessage(url, { message: msg });
    });
}

Screenshots

N/A - No UI changes

Security Check

  • Introduces new input validation
  • Modifies authentication/authorization
  • Affects data privacy/GDPR

Code Quality

  • Database: Propel ORM only, no raw SQL
  • No deprecated attributes (align, valign, nowrap, border, cellpadding, cellspacing, bgcolor)
  • Bootstrap CSS classes used
  • All CSS bundled via webpack

Pre-Merge

  • Tested locally
  • No new warnings
  • Build passes
  • Backward compatible (or migration documented)

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI mentioned this pull request Nov 26, 2025
16 tasks
Base automatically changed from fix/datepicker-verify-fix to master November 26, 2025 22:56
Co-authored-by: DawoudIO <554959+DawoudIO@users.noreply.github.com>
Copilot AI changed the title [WIP] Update financial report UX based on review feedback Refactor duplicate error handling into fetchErrorMessage helper Nov 26, 2025
Copilot AI requested a review from DawoudIO November 26, 2025 23:00
@DawoudIO DawoudIO marked this pull request as ready for review November 26, 2025 23:00
@DawoudIO DawoudIO requested a review from a team as a code owner November 26, 2025 23:00
@DawoudIO DawoudIO requested review from DAcodedBEAT, MrClever, bigtigerku, Copilot, grayeul and respencer and removed request for a team November 26, 2025 23:00
@DawoudIO DawoudIO merged commit 7cff80b into master Nov 26, 2025
6 checks passed
@DawoudIO DawoudIO deleted the copilot/sub-pr-7642 branch November 26, 2025 23:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors duplicate error handling logic in VerifyThenLoadAPIContent by extracting repeated AJAX error message fetching code into a reusable fetchErrorMessage helper function. The refactoring addresses code review feedback from PR #7642 and improves code maintainability without introducing any functional changes.

Key Changes:

  • Extracted duplicate error handling logic from 404 and 500 status code handlers into a single fetchErrorMessage helper function
  • Reduced code duplication by ~40 lines while maintaining identical behavior
Comments suppressed due to low confidence (5)

src/skin/js/CRMJSOM.js:248

  • Variable selection is used like a local variable, but is missing a declaration.
                selection = {

src/skin/js/CRMJSOM.js:259

  • Variable groupsList is used like a local variable, but is missing a declaration.
            groupsList = rdata.map(function (item) {

src/skin/js/CRMJSOM.js:268

  • Variable $groupSelect2 is used like a local variable, but is missing a declaration.
            $groupSelect2 = $("#targetGroupSelection").select2({

src/skin/js/CRMJSOM.js:282

  • Variable rolesList is used like a local variable, but is missing a declaration.
                        rolesList = rdata.map(function (item) {

src/skin/js/CRMJSOM.js:297

  • Variable params is used like a local variable, but is missing a declaration.
        params = {

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.

3 participants