microsoft / microsoft/BCApps

[Vendor] [Quality Mngmnt] - Generation Rule Management Has Unclear Separation from Traversal

Open
#6,555 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Approved Team: SCM
Dominant language
AL
Stars
683
Forks
459
Avg merge
3d 26m
Merged PRs (30d)
633

Description

Describe the issue
ISSUE: Generation Rule Management Has Unclear Separation from Traversal

Severity: HIGH  

Category: Separation of Concerns  

Files Affected:

  • app/src/Configuration/GenerationRule/QltyGenerationRuleMgmt.Codeunit.al (302 lines)

  • app/src/Configuration/SourceConfiguration/QltyTraversal.Codeunit.al

Problem:

QltyGenerationRuleMgmt codeunit depends heavily on QltyTraversal codeunit, but the division of responsibilities is unclear:


codeunit 20405 "Qlty. Generation Rule Mgmt."

{

    var

        QltyMiscHelpers: Codeunit "Qlty. Misc Helpers";

        QltyTraversal: Codeunit "Qlty. Traversal";  // Heavy dependency

    internal procedure SetFilterToApplicableTemplates(...)

    begin

        // Calls QltyTraversal.FindPossibleTargetsBasedOnConfigRecursive()

        CanLoopDoTargets := QltyTraversal.FindPossibleTargetsBasedOnConfigRecursive(...);

    end;

    procedure FindMatchingGenerationRule(...)

    begin

        // Calls QltyTraversal.FindPossibleTargetsBasedOnConfigRecursive()

        if not QltyTraversal.FindPossibleTargetsBasedOnConfigRecursive(...) then

    end;

}

Questions Raised:

  • Why is traversal separate from generation rule management?

  • Is traversal a reusable algorithm or specific to generation rules?

  • Should traversal be a local helper in GenerationRuleMgmt?

  • Or should GenerationRuleMgmt delegate more to Traversal?

Impact:

  • Unclear boundaries: Developers unsure which codeunit to modify

  • Dependency coupling: Cannot change traversal without affecting generation rules

  • Testing difficulty: Cannot test one without the other

  • Code navigation: Logic split across two codeunits increases cognitive load

Recommended Clarification:

Either merge into single cohesive unit OR clearly separate:

Option A: Merge (if traversal is only used for generation rules)


codeunit "Qlty. Generation Rule Mgmt."

{

    procedure FindMatchingGenerationRule(...)

    local procedure FindPossibleTargetsBasedOnConfigRecursive(...)

    // Traversal becomes implementation detail

}

Option B: Clear Separation (if traversal is reusable)


codeunit "Qlty. Source Config Traversal" // Generic graph traversal

{

    procedure FindTargetsRecursive(...) // Generic algorithm

    procedure GetRelatedConfigurations(...) // Generic queries

}

  

codeunit "Qlty. Generation Rule Mgmt." // Business logic

{

    procedure FindMatchingGenerationRule(...) // Uses traversal

    procedure ValidateRule(...) // Domain-specific

}

Required Investigation:

  1. Analyze all usages of QltyTraversal - is it used outside GenerationRuleMgmt?

  2. If yes → clarify interface and document as reusable component

  3. If no → merge into GenerationRuleMgmt as implementation detail

Effort Estimate: Medium (4-6 hours) - Analysis, decision, refactoring, testing

Expected behavior
Steps to reproduce
Additional context
I will provide a fix for a bug
  • I will provide a fix for a bug

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing all usages of app/src/Configuration/SourceConfiguration/QltyTraversal.Codeunit.al and comparing them with app/src/Configuration/GenerationRule/QltyGenerationRuleMgmt.Codeunit.al. Determine whether traversal is used outside generation-rule management, then document the boundary or propose the merge based on that usage. Done means the ownership decision, interface expectations, and affected tests are clear.

Written by the indexing model from the issue text.

Assessment

Domain
backend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.