Skip to content

fix(deploy-candidate): Show app name in fetch error notification#6622

Merged
siduck merged 1 commit into
developfrom
fix/deploy-candidate-app-name-in-fetch-error
Jun 5, 2026
Merged

fix(deploy-candidate): Show app name in fetch error notification#6622
siduck merged 1 commit into
developfrom
fix/deploy-candidate-app-name-in-fetch-error

Conversation

@regdocs
Copy link
Copy Markdown
Member

@regdocs regdocs commented Jun 5, 2026

When the fetch error fires from send_build_instructions_and_add_build_steps (outside the "apps" stage), the else branch showed a generic message with no app name. The GithubFetchError already carries the app name in args[1], so extract it and include it in the notification message.

When the fetch error fires from send_build_instructions_and_add_build_steps
(outside the "apps" stage), the else branch showed a generic message with no
app name. The GithubFetchError already carries the app name in args[1], so
extract it and include it in the notification message.
@regdocs regdocs requested a review from Aradhya-Tripathi as a code owner June 5, 2026 04:37
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 5, 2026

Greptile Summary

Fixes the else branch of update_with_app_not_fetchable to include the app name in the GitHub fetch-error notification, sourced from exc.args[1] (populated by GithubFetchError raise sites in app_source.py and app_release.py).

  • Extracts app_name from exc.args[1] with a safe length-guard fallback to an empty string when the arg is absent.
  • Formats the message consistently with the existing if branch, bolding the app name when available and falling back to the generic "App " prefix otherwise.

Confidence Score: 5/5

One-line change in a notification helper; no data mutations, no ORM calls, no permission paths touched.

Both raise sites of GithubFetchError pass self.app (an internal app slug) as args[1], so the length guard and the HTML interpolation are safe. The fallback to 'App ' correctly handles any future raise site that omits the second arg. The change is fully consistent with how the existing if branch already formats the same message.

No files require special attention.

Important Files Changed

Filename Overview
press/press/doctype/deploy_candidate/deploy_notifications.py Extracts app_name from exc.args[1] in the else branch so the notification message names the failing app; falls back to 'App' when args[1] is absent.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[update_with_app_not_fetchable called] --> B{stage_slug == apps?}
    B -- Yes --> C[app_name = failed_step.step]
    B -- No --> D{exc.args has 2+ items?}
    D -- Yes --> E[app_name = exc.args 1]
    D -- No --> F[app_name = empty string]
    C --> G[Message: bold app_name could not be fetched]
    E --> H{app_name truthy?}
    F --> H
    H -- Yes --> I[app_str = bold app_name]
    H -- No --> J[app_str = App]
    I --> K[Message: app_str could not be fetched from GitHub]
    J --> K
    K --> L[Set details message and assistance_url]
    G --> L
Loading

Reviews (1): Last reviewed commit: "fix(deploy-candidate): Show app name in ..." | Re-trigger Greptile

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 5, 2026

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.17%. Comparing base (666028d) to head (70684d6).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
...s/doctype/deploy_candidate/deploy_notifications.py 0.00% 3 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6622      +/-   ##
===========================================
- Coverage    50.18%   50.17%   -0.01%     
===========================================
  Files          990      990              
  Lines        82967    82969       +2     
  Branches       523      523              
===========================================
  Hits         41633    41633              
- Misses       41302    41304       +2     
  Partials        32       32              
Flag Coverage Δ
dashboard 62.80% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@siduck siduck merged commit 2ec2e30 into develop Jun 5, 2026
14 checks passed
@siduck siduck deleted the fix/deploy-candidate-app-name-in-fetch-error branch June 5, 2026 05:36
@siduck
Copy link
Copy Markdown
Collaborator

siduck commented Jun 5, 2026

@mergify backport master

@siduck siduck restored the fix/deploy-candidate-app-name-in-fetch-error branch June 5, 2026 05:36
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Jun 5, 2026

backport master

✅ Backports have been created

Details

siduck added a commit that referenced this pull request Jun 5, 2026
fix(deploy-candidate): Show app name in fetch error notification (backport #6622)
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