Treat non-applicable keys as defaults which then get overridden

[gilluminate]

Closes LJ-720

Description Of Changes

Fixed a bug in the GTM integration where duplicate notice keys between privacy_notices and non_applicable_privacy_notices arrays caused incorrect consent values to be reported. The issue occurred when custom notices were created with the same notice_key as existing notices.

Code Changes

GTM Integration Fix: Completely reworked how consent values are initialized and applied in the GTM integration. Now it:

1. First initializes an empty consent values object
2. Sets defaults for non-applicable notices when needed
3. Then overrides with actual consent values from configured notices
4. This ensures proper prioritization even when duplicate keys exist

Admin UI Preview Improvement: Replaced the use of systemsCount with a proper vendorCount that is fetched from the vendor report API, providing more accurate vendor count data for previews.

Steps to Confirm

1. Create a custom notice with the same notice_key as an existing notice (e.g., "data_sales_and_sharing")
2. Add only the custom notice to an experience
3. Verify the GTM integration correctly reports the consent value from the custom notice, not the default from the non-applicable notice
4. Confirm no duplicates appear in the dataLayer object

Pre-Merge Checklist

• Issue requirements met
• All CI pipelines succeeded
• CHANGELOG.md updated
  • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
  • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
  • Updates unreleased work already in Changelog, no new entry necessary
• Followup issues:
  • Followup issues created
  • No followup issues
• Database migrations:
  • Ensure that your downrev is up to date with the latest revision on main
  • Ensure that your downgrade() migration is correct and works
    • If a downgrade migration is not possible for this change, please call this out in the PR description!
  • No migrations
• Documentation:
  • Documentation complete, PR opened in fidesdocs
  • Documentation issue created in fidesdocs
  • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
  • No documentation updates required

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
fides-privacy-center	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 24, 2025 10:59pm

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
fides-plus-nightly	⬜️ Ignored (Inspect)	Visit Preview		Apr 24, 2025 10:59pm

[tvandort]

Small nit but I feel like we'd be better served by reducing the overall cyclomatic complexity of this code.

Instead of wrapping this in an if statement we could:

const nextConsent = consent ?? {}
if (privacyNotices && flagType === GtmFlagType.CONSENT_MECHANISM) {
  Object.entries(nextConsent).forEach(([key, value]) => {
    consentValues[key] = transformConsentToFidesUserPreference(
      value,
      privacyNotices.find((notice) => notice.notice_key === key)
        ?.consent_mechanism,
    );
  });
} else {
  Object.entries(nextConsent).forEach(([key, value]) => {
    consentValues[key] = value;
  });
}

[gilluminate]

if there's no consent, it doesn't do anything else, which I think is less cyclomatic, no?

[gilluminate]

Additionally, I'm betting the rollup algorithm is going to make this nice and clean anyway, and I'd favor readability in the source code.

[tvandort]

I find less if statements (well... really less execution branches) more readable personally. :)

Something that always happens is easier to reason about than something that sometimes happens.

if there's no consent, it doesn't do anything else, which I think is less cyclomatic, no?

No I don't believe so. At least to my understand cyclomatic complexity goes up with mutually exclusive execution paths in the code. If a path always executes it's less complex than one that does.

Let's take a simpler example case:

function trim(str) {
	if (str) {
		return str.trim();
	}
	
	return "";
}

This code has two paths, one where a another function, .trim() will be executed and one where it will not. CC of 2.

function trim(str) {
	var trimmedStr = str ?? "";
	return trimmedStr.trim();
}

This code has one path, .trim() is always called. Therefore (assuming I understand correctly) it has a lower cyclomatic complexity. CC of 1.

[cypress]

fides    Run #12877

Run Properties:   Passed #12877  •  ff60e765f6: Treat non-applicable keys as defaults which then get overridden (#6090)

Project	fides
Branch Review	main
Run status	Passed #12877
Run duration	00m 52s
Commit	ff60e765f6: Treat non-applicable keys as defaults which then get overridden (#6090)
Committer	Jason Gill
View all properties for this run ↗︎

---

Test results
Failures	0
Flaky	0
Pending	0
Skipped	0
Passing	5
⚠️ You've recorded test results over your free plan limit. Upgrade your plan to view test results.
View all changes introduced in this branch ↗︎