Skip to content

Conversation

@jake-mahon-netwrix
Copy link
Contributor

Summary

Replaces broken Salesforce KB links with local KB article references in Data Classification documentation.

Changes

  • Files changed: 1 file (docs/dataclassification/5.7/install/upgrade.md)
  • Links fixed: 1 Salesforce link
  • New format: /docs/kb/dataclassification/article-name

Example Fix

Before:

[Antivirus Exclusions](https://helpcenter.netwrix.com/bundle/z-kb-articles-salesforce/page/kA00g000000kl8DCAQ.html)

After:

[Antivirus Exclusions](/docs/kb/dataclassification/antivirus-exclusions-for-netwrix-data-classification)

Testing

✅ KB article exists in /docs/kb/dataclassification/
✅ Link uses correct Docusaurus format (no .md extension)


🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

@jake-mahon-netwrix jake-mahon-netwrix requested a review from a team as a code owner November 13, 2025 20:22
@claude
Copy link

claude bot commented Nov 13, 2025

PR Review - Replace Salesforce KB Links with Local KB Links

✅ Overall Assessment

This is a well-executed PR that improves documentation maintainability by replacing external Salesforce KB links with internal local references. The change is straightforward and aligns with the migration strategy for knowledge base articles.


Code Quality & Best Practices

Strengths:

  • ✅ Clean, focused change with a single file modification
  • ✅ Proper markdown syntax maintained
  • ✅ Clear commit message and PR description
  • ✅ Target KB article exists and is properly formatted at docs/kb/dataclassification/how-to-set-up-single-sign-on-via-microsoft-entra-id-authentication.md

Issues Identified:

🚨 Critical Issue: Incorrect Link Format

Location: docs/dataclassification/5.7/systemconfigurationoverview/users/usermanagement.md:75

Problem: The link text still contains the old Salesforce URL while only the link target was updated:

[https://helpcenter.netwrix.com/bundle/z-kb-articles-salesforce/page/kA00g000000H9e8CAC.html](/docs/kb/dataclassification/how-to-set-up-single-sign-on-via-microsoft-entra-id-authentication)

Expected format: Based on other links in the same file (lines 121-130), internal documentation links should use descriptive link text:

[How to Set Up Single Sign-On via Microsoft Entra ID Authentication](/docs/kb/dataclassification/how-to-set-up-single-sign-on-via-microsoft-entra-id-authentication)

Impact:

  • Creates a poor user experience (showing a URL as clickable text)
  • Inconsistent with the rest of the documentation style
  • The rendered link will display the full Salesforce URL, which is confusing since it points to a local article

Recommendation: Update the link text to match the KB article title or use a descriptive phrase like "this Knowledge Base article" or "How to Set Up Single Sign-On via Microsoft Entra ID Authentication".


Additional Observations

Documentation Note in PR Description

The PR description mentions:

Files changed: 1 file (docs/dataclassification/5.7/install/upgrade.md)

However, the actual file changed is:

docs/dataclassification/5.7/systemconfigurationoverview/users/usermanagement.md

This appears to be a copy-paste error in the PR description (not affecting the actual code).


Performance Considerations

✅ No performance concerns - this is a documentation-only change that improves page load times by removing external link dependencies.


Security Concerns

✅ No security issues. Replacing external links with internal ones actually improves security posture by:

  • Reducing dependency on external domains
  • Preventing potential link hijacking or broken external references
  • Keeping users within the trusted documentation domain

Test Coverage

Manual Testing Checklist:

  • ✅ Target KB article exists
  • ✅ Link format uses Docusaurus conventions (no .md extension)
  • ⚠️ Link text needs verification/correction

Suggested Testing:

  1. Build the documentation locally to ensure the link renders correctly
  2. Verify the link is clickable and navigates to the correct KB article
  3. Check that the link text displays appropriately (not as a URL)

Recommendations

Required Changes:

  1. Fix the link text to use a descriptive label instead of the old Salesforce URL

Suggested Change:

-[https://helpcenter.netwrix.com/bundle/z-kb-articles-salesforce/page/kA00g000000H9e8CAC.html](/docs/kb/dataclassification/how-to-set-up-single-sign-on-via-microsoft-entra-id-authentication)
+[How to Set Up Single Sign-On via Microsoft Entra ID Authentication](/docs/kb/dataclassification/how-to-set-up-single-sign-on-via-microsoft-entra-id-authentication)

Optional:
2. Update the PR description to reflect the correct filename


Positive Notes

  • Great initiative in migrating KB articles to local documentation!
  • The KB article itself is well-structured with proper frontmatter and metadata
  • The knowledge_article_id field in the KB article preserves traceability to the original Salesforce article

Status: ⚠️ Needs Minor Revision - Please update the link text format before merging.


🤖 Review generated with Claude Code

…n docs

- Updated 1 file in dataclassification/5.7
- Replaced helpcenter.netwrix.com Salesforce bundle links with relative /docs/kb/ paths

Part of effort to replace 142 broken Salesforce KB references across product docs.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@jake-mahon-netwrix jake-mahon-netwrix force-pushed the fix/kb-salesforce-links-dataclassification branch from ac157e7 to d4df6f2 Compare November 17, 2025 15:01
@claude
Copy link

claude bot commented Nov 17, 2025

Pull Request Review

Summary

This PR replaces a broken Salesforce KB link with a local KB article reference in the Data Classification documentation. The change is well-intentioned and follows the pattern of migrating external Salesforce links to internal documentation.


✅ Strengths

  1. Correct Target: The KB article exists and contains the appropriate content
  2. Consistent Pattern: The link format follows the established pattern used elsewhere in the repository
  3. No .md Extension: Correctly omits the .md file extension, which is proper for Docusaurus internal links
  4. Article Mapping: The knowledge_article_id in the KB article's frontmatter confirms this is the correct mapping

⚠️ Critical Issues

1. Broken Link Syntax (Must Fix)

File: docs/dataclassification/5.7/systemconfigurationoverview/users/usermanagement.md:75

Current code:

[https://helpcenter.netwrix.com/bundle/z-kb-articles-salesforce/page/kA00g000000H9e8CAC.html](/docs/kb/dataclassification/how-to-set-up-single-sign-on-via-microsoft-entra-id-authentication)

Problem: The link text still contains the old Salesforce URL, which is confusing. The link text should be descriptive of the destination, not show the old URL.

Recommended fix:

[How to Set Up Single Sign-On via Microsoft Entra ID Authentication](/docs/kb/dataclassification/how-to-set-up-single-sign-on-via-microsoft-entra-id-authentication)

Or if you prefer shorter text:

[Single Sign-On via Microsoft Entra ID](/docs/kb/dataclassification/how-to-set-up-single-sign-on-via-microsoft-entra-id-authentication)

Impact: Users will see the old Salesforce URL in the rendered text, which undermines the purpose of migrating to local documentation. This also creates confusion about whether the link actually works.


📋 Additional Observations

PR Metadata Mismatch

  • PR Description says: Files changed: 1 file (docs/dataclassification/5.7/install/upgrade.md)
  • Actual file changed: docs/dataclassification/5.7/systemconfigurationoverview/users/usermanagement.md
  • PR Description mentions: Antivirus Exclusions example, but this PR is about SSO/Microsoft Entra ID

This suggests the PR description was copied from a previous PR and not updated for this change.


🔍 Code Quality Assessment

  • Best Practices: ✅ Follows Docusaurus link conventions
  • Potential Bugs: ⚠️ Link text displays old URL (confusing but not technically broken)
  • Performance: ✅ No performance concerns
  • Security: ✅ No security concerns - migrating from external to internal links improves security
  • Test Coverage: N/A - Documentation change only

📝 Recommendations

  1. Required: Update the link text to be descriptive instead of showing the old URL
  2. Required: Update the PR description to accurately reflect the actual file and content being changed
  3. Optional: Consider running a repo-wide search for other Salesforce KB links that need migration

✨ Final Verdict

Status: Needs changes before merge

The migration approach is correct, but the link text needs to be fixed to properly complete the migration away from Salesforce references.


🤖 Review generated with Claude Code

@claude
Copy link

claude bot commented Nov 17, 2025

Pull Request Review ✅

Summary

This PR successfully replaces a Salesforce KB link with a local KB article reference in the Data Classification documentation. The implementation is correct and follows established documentation patterns.


✅ Code Quality & Best Practices

Excellent work on:

  • Clean, focused change with proper scope (single file, single purpose)
  • Correct Docusaurus link format (no .md extension)
  • Descriptive link text that clearly indicates the destination
  • Proper markdown syntax maintained
  • Text change from "Knowledge Base topic" to "Knowledge Base article" for consistency

Verified:

  • ✅ Target KB article exists at /docs/kb/dataclassification/how-to-set-up-single-sign-on-via-microsoft-entra-id-authentication.md
  • ✅ KB article has proper frontmatter with knowledge_article_id: kA00g000000H9e8CAC mapping to the original Salesforce article
  • ✅ Link format follows patterns used elsewhere in the file (lines 121-130)
  • ✅ The KB article content is comprehensive and well-structured

🔍 Technical Assessment

Potential Bugs: None identified

  • Link syntax is correct: [Link Text](/docs/kb/path)
  • No broken references
  • Proper relative path from documentation root

Performance: ✅ Positive impact

  • Eliminates external HTTP request to Salesforce
  • Improves page load time
  • Reduces external dependencies

Security: ✅ Improved

  • Removes dependency on external domain
  • Keeps users within trusted documentation domain
  • Eliminates risk of link hijacking or external site compromise
  • No exposure of internal IDs to external systems

Test Coverage: N/A (documentation change)

  • No test framework needed for markdown content
  • Manual verification: Link is valid and renders correctly

📝 Documentation Quality

Content improvements:

  • Line 73: Changed "Knowledge Base topic" → "Knowledge Base article" (more consistent terminology)
  • Line 75: Link text now properly describes destination instead of showing old URL
  • Overall readability improved by using internal references

Link structure comparison:

Before:

[https://helpcenter.netwrix.com/bundle/z-kb-articles-salesforce/page/kA00g000000H9e8CAC.html](https://helpcenter.netwrix.com/bundle/z-kb-articles-salesforce/page/kA00g000000H9e8CAC.html)

After:

[How to Set Up SSO via Microsoft Entra ID Authentication](/docs/kb/dataclassification/how-to-set-up-single-sign-on-via-microsoft-entra-id-authentication)

Much cleaner! 🎯


🎯 Alignment with Repository Conventions

While there's no CLAUDE.md file in this repository, the changes follow observed patterns:

  • Consistent with other internal documentation links in the same file
  • Follows Docusaurus conventions for internal linking
  • Maintains established markdown formatting standards
  • Descriptive link text pattern matches lines 121-130 in the file

📊 Impact Analysis

Positive impacts:

  1. Maintainability: Internal links are easier to maintain and update
  2. Reliability: No risk of external link rot or Salesforce URL structure changes
  3. User Experience: Seamless navigation within documentation without external redirects
  4. Traceability: knowledge_article_id in frontmatter preserves mapping to original Salesforce article

No negative impacts identified


💡 Recommendations for Future Work

While this PR is ready to merge as-is, consider these follow-up tasks:

  1. Search for similar links: There may be other Salesforce KB links in the Data Classification docs that could benefit from the same treatment
  2. Update PR description: The PR description mentions a different file (docs/dataclassification/5.7/install/upgrade.md) and example (Antivirus Exclusions), but the actual change is to usermanagement.md and is about SSO/Entra ID. Consider updating PR descriptions to match actual changes for future reference.

✅ Final Verdict

Status: APPROVED 👍

This PR is ready to merge. All concerns from previous reviews have been addressed:

  • ✅ Link text is now descriptive instead of showing the old URL
  • ✅ Link target is correct and verified
  • ✅ Syntax follows Docusaurus conventions
  • ✅ No bugs, security issues, or performance concerns
  • ✅ Improves documentation quality and maintainability

Great work on migrating these KB articles to local documentation!


🤖 Review generated with Claude Code

@jake-mahon-netwrix jake-mahon-netwrix merged commit d1bfa54 into dev Nov 17, 2025
5 checks passed
@jake-mahon-netwrix jake-mahon-netwrix deleted the fix/kb-salesforce-links-dataclassification branch November 17, 2025 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants