feat(job): print dashboard preview link

[kannwism]

Summary

• print a dashboard preview URL after creating a job definition
• keep the existing phone QR code preview unchanged

Validation

• git diff --check
• PATH="/home/agent/.local/share/mise/installs/node/20.20.2/bin:$PATH" pyright src/rapidata/rapidata_client (fails: dependencies are not installed in this container, causing repo-wide missing-import errors for pydantic, pandas, opentelemetry, etc.)

🔗 Session: session-56ddff8d

[claude]

Code Review

Overview

This PR adds a dashboard URL preview link printed to the terminal after creating a job definition. The implementation is clean and minimal - two small functions in _qr_preview.py and a single call site in _create_general_job_definition.

---

What works well

• Respects silent_mode, consistent with all other print helpers in this module.
• Uses managed_print and follows the same pattern as the existing campaign preview helpers.
• Good separation between the URL builder (build_job_definition_preview_url) and the printing function - the builder is independently testable and reusable.
• No unnecessary abstraction or over-engineering.

---

Issues / Suggestions

1. Module docstring is now stale

The module-level docstring at the top of _qr_preview.py still reads:

Helpers for printing a terminal-friendly QR code that links to the public campaign preview page.

Now that the module also handles plain URL link printing for job definitions, this should be updated to reflect the broader scope.

---

2. No error handling - inconsistent with the module's stated contract

The module docstring explicitly promises best-effort semantics:

failures are logged at debug level and never raised to the caller, so order / job execution is never affected

print_job_definition_preview_link is the only print helper in the file that does not wrap its body in a try/except. While managed_print on a formatted string is extremely unlikely to fail, it still breaks the contract stated for this module. For consistency, the body should be wrapped in a try/except with logger.debug on failure, matching the pattern used throughout the rest of the module.

---

3. Print message wording inconsistency

The fallback in print_campaign_preview_qr uses "Preview at: {url}", while the new function uses "Preview in dashboard: {url}". The difference may be intentional (different surfaces), but it is worth confirming this is a deliberate UX choice and not an oversight.

---

4. Ordering: QR code then link

In _create_general_job_definition, the campaign QR is printed before the dashboard link. The QR lookup involves retries via _resolve_campaign_id_from_pipeline, so there can be a noticeable delay before the dashboard link appears. Worth verifying the UX feels coherent in practice.

---

5. Pyright could not be validated

The PR notes pyright could not run due to missing deps in the container. The key type to verify is job_model.id - if RapidataJobDefinition.id is typed as str | None, passing it to build_job_definition_preview_url(job_definition_id: str) would be a type error. Worth confirming this locally before merging.

---

Summary

Small, clean addition. The main gap is the missing try/except wrapper (for consistency with the module's own best-effort contract) and the stale module docstring. Everything else looks good.

[LinoGiger]

What about the order?

[claude]

Code Review — feat(job): print dashboard preview link

Overview

This PR adds a text dashboard URL after job definition and order creation, complementing the existing phone-oriented QR code preview. The change is small, focused, and well-scoped.

---

What's Good

• Consistent patterns: The new functions follow the exact same structure as print_campaign_preview_qr — silent-mode guard, delegate to a build_* helper, call managed_print. Easy to read and maintain.
• No error risk: Unlike the QR renderer (which wraps an optional qrcode import), these functions only do string formatting and a managed print — no try/except needed.
• Correct placement: In both _create_general_job_definition and _create, the new link is printed after the existing QR code, so the terminal output ordering is phone-preview first, dashboard link second. That's a natural UX order.

---

Issues / Suggestions

1. build_job_definition_preview_url / build_order_preview_url are public but only used internally

Both builder helpers are reachable from outside the module (build_* without a leading _), but they're only called by their corresponding print_* wrappers. Unless you intend to expose them as a public utility, prefix them with _ to keep the module surface minimal:

# Before
def build_job_definition_preview_url(environment: str, job_definition_id: str) -> str:
# After
def _build_job_definition_preview_url(environment: str, job_definition_id: str) -> str:

The existing build_campaign_preview_url is already public, but that one is used by print_campaign_preview_qr and directly tested elsewhere — so there may be symmetry intent here. If you want them public for external callers (e.g. notebooks, custom logging), that's fine — just worth a deliberate choice.

2. Duplicate message text

Both new print functions use the identical string "Preview in dashboard: {url}". If this label ever needs to change you'll need to update two places. Consider a shared constant:

_DASHBOARD_PREVIEW_MSG = "Preview in dashboard: {url}"

Minor point — not a blocker.

3. No test coverage

No tests exist for any of the _qr_preview.py helpers (including the originals), so this is consistent with the rest of the module. Still worth noting: the new URL templates are untested, so a typo in "https://app.{environment}/definitions/{job_definition_id}" would silently produce a bad URL. A quick parametrised unit test asserting the formatted output would catch that.

4. _APP_ORDER_URL_TEMPLATE singular vs. plural

The job-definition path is /definitions/ (plural) but the order path is /order/detail/ (singular). This is almost certainly intentional to match actual app routes, but worth a quick sanity-check that the live URL /order/detail/<id> is correct and not /orders/detail/<id>.

---

Summary

The implementation is clean and correct. The two items worth acting on before merge are:

1. Verify the /order/detail/ URL path is accurate (easy to check, high impact if wrong).
2. Consider making build_* helpers private if they're not meant to be part of the public API.

Everything else is minor style/future-proofing. Good PR overall.