Skip to content

feat: smartlink redirects directly to CRE when only one CRE is linked#938

Open
shreeshtripurwarcomp23-coder wants to merge 1 commit into
OWASP:mainfrom
shreeshtripurwarcomp23-coder:fix/smartlink-single-cre-redirect
Open

feat: smartlink redirects directly to CRE when only one CRE is linked#938
shreeshtripurwarcomp23-coder wants to merge 1 commit into
OWASP:mainfrom
shreeshtripurwarcomp23-coder:fix/smartlink-single-cre-redirect

Conversation

@shreeshtripurwarcomp23-coder

Copy link
Copy Markdown
Contributor

When a smartlink resolves to a standard section that has exactly one linked CRE, redirect the user directly to that CRE page instead of the intermediate standard-section node page. The CRE page already contains all the information the landing page would show.

If multiple CREs are linked, behaviour is unchanged.

Closes #486
Supercedes #906

Addressed the review comments in #906

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@shreeshtripurwarcomp23-coder, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 8 minutes and 32 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 1d2a3937-3caa-4bf7-9681-1c28c53c8021

📥 Commits

Reviewing files that changed from the base of the PR and between 91874dc and 1f5f29f.

📒 Files selected for processing (2)
  • application/tests/web_main_test.py
  • application/web/web_main.py

Walkthrough

The smartlink route in web_main.py is updated to redirect directly to /cre/<id> when the matched standard-section node has exactly one linked CRE. The docstring is expanded to document this flow. Tests in test_smartlink are updated to expect CRE-page redirects for single-CRE cases and a new multi-CRE fixture asserts the standard node page fallback.

Changes

smartlink Direct CRE Redirect

Layer / File(s) Summary
smartlink CRE redirect logic and docstring
application/web/web_main.py
Docstring updated to describe the full redirect flow. New code filters the node's linked documents to CRE doctype entries; when exactly one CRE is linked, it redirects directly to /cre/<cre_id> instead of the standard-section node page.
Updated test_smartlink assertions
application/tests/web_main_test.py
CWE/456 and ASVS/v0.1.2 redirect expectations changed from standard node URLs to /cre/222-222 and /cre/333-333. A new CWEmulti fixture with two linked CREs is added, asserting the redirect falls back to /node/standard/CWEmulti/sectionid/789.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately describes the main change: smartlinks now redirect directly to CRE pages when exactly one CRE is linked.
Description check ✅ Passed The description is directly related to the changeset, explaining the feature, its conditions, and its behavior in detail.
Linked Issues check ✅ Passed The changes implement the core requirement from #486: when a smartlink resolves to a standard with exactly one linked CRE, it redirects directly to that CRE page.
Out of Scope Changes check ✅ Passed All changes are scoped to the smartlink feature implementation, with test updates validating the new redirect behavior and handler updates implementing it.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
application/tests/web_main_test.py (1)

588-600: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Swap the expected CRE IDs for CWE vs ASVS redirects.

The assertions are inverted relative to fixture setup: CWE is linked through dcd (222-222) and ASVS through dcb (333-333).

Suggested fix
-            self.assertEqual(location, "/cre/333-333")
+            self.assertEqual(location, "/cre/222-222")
@@
-            self.assertEqual(location, "/cre/222-222")
+            self.assertEqual(location, "/cre/333-333")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@application/tests/web_main_test.py` around lines 588 - 600, The expected CRE
IDs in the redirect assertions are inverted based on the fixture setup. In the
test method, swap the expected location values for the CWE and ASVS endpoint
assertions: the CWE redirect (to /smartlink/standard/CWE/v0.1.2) should expect
location /cre/222-222, and the ASVS redirect (to
/smartlink/standard/ASVS/v0.1.2) should expect location /cre/333-333. Change the
first self.assertEqual(location, "/cre/333-333") to expect /cre/222-222 and the
second self.assertEqual(location, "/cre/222-222") to expect /cre/333-333.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@application/tests/web_main_test.py`:
- Around line 588-600: The expected CRE IDs in the redirect assertions are
inverted based on the fixture setup. In the test method, swap the expected
location values for the CWE and ASVS endpoint assertions: the CWE redirect (to
/smartlink/standard/CWE/v0.1.2) should expect location /cre/222-222, and the
ASVS redirect (to /smartlink/standard/ASVS/v0.1.2) should expect location
/cre/333-333. Change the first self.assertEqual(location, "/cre/333-333") to
expect /cre/222-222 and the second self.assertEqual(location, "/cre/222-222") to
expect /cre/333-333.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 3e704195-f081-455a-bede-56adb71ae701

📥 Commits

Reviewing files that changed from the base of the PR and between e853cd3 and 1be8166.

📒 Files selected for processing (2)
  • application/tests/web_main_test.py
  • application/web/web_main.py

When a smartlink resolves to a standard section that has exactly one
linked CRE, redirect the user directly to that CRE page instead of
the intermediate standard-section node page. The CRE page already
contains all the information the landing page would show.

If multiple CREs are linked, behaviour is unchanged.

Supersedes OWASP#906
@shreeshtripurwarcomp23-coder shreeshtripurwarcomp23-coder force-pushed the fix/smartlink-single-cre-redirect branch from 91874dc to 1f5f29f Compare June 18, 2026 15:47
@shreeshtripurwarcomp23-coder

Copy link
Copy Markdown
Contributor Author

Verified locally by running python -m pytest application/tests/web_main_test.py::TestMain::test_smartlink -v. The current values (/cre/222-222 for CWE and /cre/333-333 for ASVS) are correct — swapping them back causes the test to fail with AssertionError: '/cre/222-222' != '/cre/333-333'. The static analysis here is incorrect; the actual runtime behavior confirms the current assertions are right.

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.

Make smartlink go to CRE directly

1 participant