Add HTML imgbed upload support#162
Open
clown145 wants to merge 4 commits intoSXP-Simon:mainfrom
Open
Conversation
Contributor
Reviewer's GuideAdds an HtmlReportPublisher to centralize HTML report delivery and introduce optional upload to an HTML imgbed (CloudFlare-ImgBed style), with corresponding config options and README documentation updates. Sequence diagram for HTML report publishing with optional imgbed uploadsequenceDiagram
participant G as GroupDailyAnalysis
participant RD as ReportDispatcher
participant HRP as HtmlReportPublisher
participant CM as ConfigManager
participant MS as MessageSender
participant IB as ImgBedServer
G->>HRP: publish(group_id, html_path, platform_id)
activate HRP
HRP->>CM: get_html_upload_enabled()
CM-->>HRP: enabled_flag
alt upload_enabled
HRP->>HRP: upload(html_path)
activate HRP
HRP->>CM: get_html_base_url(), get_html_upload_token(), get_html_upload_channel()
CM-->>HRP: base_url, token, channel
HRP->>IB: POST /upload (file, token, channel)
IB-->>HRP: JSON payload (src or error)
HRP->>HRP: _extract_src(payload)
HRP-->>HRP: public_url or None
deactivate HRP
alt upload_success
HRP->>HRP: build_caption(public_url=public_url)
HRP->>MS: send_text(group_id, caption, platform_id)
MS-->>HRP: sent_true_or_false
alt text_sent_ok
HRP-->>G: True
else text_send_failed
HRP->>MS: send_file(group_id, html_path, caption, platform_id)
MS-->>HRP: sent_result
HRP-->>G: sent_result
end
else upload_failed
HRP->>MS: send_file(group_id, html_path, caption, platform_id)
MS-->>HRP: sent_result
HRP-->>G: sent_result
end
else upload_disabled
HRP->>HRP: build_caption(html_path=html_path)
HRP->>MS: send_file(group_id, html_path, caption, platform_id)
MS-->>HRP: sent_result
HRP-->>G: sent_result
end
Class diagram for HTML report publishing with HtmlReportPublisherclassDiagram
class HtmlReportPublisher {
- config_manager
- message_sender
+ HtmlReportPublisher(config_manager, message_sender)
+ build_caption(html_path, public_url) str
+ publish(group_id, html_path, platform_id) bool
+ upload(html_path) str
- _normalized_base_url() str
- _build_self_hosted_url(html_path) str
- _build_public_url(src) str
- _extract_src(payload) str
}
class ConfigManager {
+ get_html_base_url() str
+ get_html_upload_enabled() bool
+ get_html_upload_token() str
+ get_html_upload_channel() str
+ get_html_output_dir() str
}
class MessageSender {
+ send_text(group_id, text, platform_id) bool
+ send_file(group_id, file_path, caption, platform_id) bool
}
class ReportDispatcher {
- config_manager
- report_generator
- message_sender
- html_report_publisher
+ ReportDispatcher(config_manager, report_generator, message_sender)
+ set_html_render(render_func)
+ dispatch(...)
- _dispatch_html(group_id, platform_id, trace_context)
}
class GroupDailyAnalysis {
- config_manager
- bot_manager
- analysis_service
- report_generator
- message_sender
- html_report_publisher
+ GroupDailyAnalysis(context, config)
+ some_handler_methods()
}
HtmlReportPublisher --> ConfigManager : uses
HtmlReportPublisher --> MessageSender : uses
ReportDispatcher --> HtmlReportPublisher : composes
GroupDailyAnalysis --> HtmlReportPublisher : composes
ReportDispatcher --> MessageSender
GroupDailyAnalysis --> MessageSender
ConfigManager <.. ReportDispatcher
ConfigManager <.. GroupDailyAnalysis
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- When
html_upload_enabledis true and the upload fails,build_captionwill never include a self-hostedhtml_base_urllink because it explicitly skips_build_self_hosted_urlin that mode; consider basing the decision on whetherpublic_urlis present instead of thehtml_upload_enabledflag so the caption still contains a usable link on fallback. - In
_build_public_url, absolutesrcvalues (starting withhttp://orhttps://) are forcibly remapped underhtml_base_url, which can break correct external URLs returned by some providers; it may be safer to return absolute URLs as-is and only joinhtml_base_urlfor relative paths. - In
upload, the broadexcept Exception as eonly logs the message string; switching tologger.exception(or includingexc_info=True) would make diagnosing upload issues easier by recording stack traces.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When `html_upload_enabled` is true and the upload fails, `build_caption` will never include a self-hosted `html_base_url` link because it explicitly skips `_build_self_hosted_url` in that mode; consider basing the decision on whether `public_url` is present instead of the `html_upload_enabled` flag so the caption still contains a usable link on fallback.
- In `_build_public_url`, absolute `src` values (starting with `http://` or `https://`) are forcibly remapped under `html_base_url`, which can break correct external URLs returned by some providers; it may be safer to return absolute URLs as-is and only join `html_base_url` for relative paths.
- In `upload`, the broad `except Exception as e` only logs the message string; switching to `logger.exception` (or including `exc_info=True`) would make diagnosing upload issues easier by recording stack traces.
## Individual Comments
### Comment 1
<location path="src/infrastructure/reporting/html_publisher.py" line_range="182-189" />
<code_context>
+ return None
+
+ src = str(src).strip()
+ if src.startswith(("http://", "https://")):
+ parsed = urlsplit(src)
+ relative = parsed.path or "/"
+ if parsed.query:
+ relative = f"{relative}?{parsed.query}"
+ if parsed.fragment:
+ relative = f"{relative}#{parsed.fragment}"
+ return (
+ f"{base_url}{relative if relative.startswith('/') else '/' + relative}"
+ )
</code_context>
<issue_to_address>
**question (bug_risk):** Rewriting absolute `src` URLs with `html_base_url` may be surprising if the uploader returns fully qualified public URLs.
In `_build_public_url`, absolute `src` values are re-mapped onto `html_base_url`, dropping their original scheme/host and keeping only path/query/fragment. This works if the uploader returns internal URLs that must be exposed via a public gateway, but will break setups where the uploader already returns public URLs. Consider preserving absolute URLs as-is and only applying `html_base_url` to relative paths.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+182
to
+189
| if src.startswith(("http://", "https://")): | ||
| parsed = urlsplit(src) | ||
| relative = parsed.path or "/" | ||
| if parsed.query: | ||
| relative = f"{relative}?{parsed.query}" | ||
| if parsed.fragment: | ||
| relative = f"{relative}#{parsed.fragment}" | ||
| return ( |
Contributor
There was a problem hiding this comment.
question (bug_risk): Rewriting absolute src URLs with html_base_url may be surprising if the uploader returns fully qualified public URLs.
In _build_public_url, absolute src values are re-mapped onto html_base_url, dropping their original scheme/host and keeping only path/query/fragment. This works if the uploader returns internal URLs that must be exposed via a public gateway, but will break setups where the uploader already returns public URLs. Consider preserving absolute URLs as-is and only applying html_base_url to relative paths.
Contributor
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd86f47554
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+88
to
+89
| if channel and channel != "default": | ||
| params["uploadChannel"] = channel |
There was a problem hiding this comment.
default to a concrete upload channel
When html_upload_channel is left as default, this code intentionally omits uploadChannel from the /upload request. In CloudFlare-ImgBed's API, the documented default for uploadChannel is telegram, so deployments that only configure cfr2/s3/discord/etc. can fail uploads and always fall back to local-file sending despite upload mode being enabled. This should map default to an actual available channel (or validate against /api/channels) instead of silently dropping the parameter.
Useful? React with 👍 / 👎.
Owner
|
Preparing review... |
1 similar comment
Owner
|
Preparing review... |
Owner
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Owner
|
PR Description updated to latest commit (ac7a0e4) |
Owner
PR Code Suggestions ✨
|
Owner
|
PR Description updated to latest commit (ac7a0e4) |
SXP-Simon
reviewed
Apr 27, 2026
Comment on lines
+176
to
+193
| def _build_public_url(self, src: str) -> str | None: | ||
| base_url = self._normalized_base_url() | ||
| if not base_url or not src: | ||
| return None | ||
|
|
||
| src = str(src).strip() | ||
| if src.startswith(("http://", "https://")): | ||
| parsed = urlsplit(src) | ||
| relative = parsed.path or "/" | ||
| if parsed.query: | ||
| relative = f"{relative}?{parsed.query}" | ||
| if parsed.fragment: | ||
| relative = f"{relative}#{parsed.fragment}" | ||
| return ( | ||
| f"{base_url}{relative if relative.startswith('/') else '/' + relative}" | ||
| ) | ||
|
|
||
| return f"{base_url}/{src.lstrip('/')}" |
Owner
There was a problem hiding this comment.
Suggestion: When the “src” returned by the image bed is an absolute URL (e.g., https://imgbed.example.com/files/abc.html), the current logic strips the host and prepends base_url again, which can produce a broken link. Instead, return the absolute src unchanged, since the base URL is already configured as the image bed’s root. [possible issue, importance: 8]
Suggested change
| def _build_public_url(self, src: str) -> str | None: | |
| base_url = self._normalized_base_url() | |
| if not base_url or not src: | |
| return None | |
| src = str(src).strip() | |
| if src.startswith(("http://", "https://")): | |
| parsed = urlsplit(src) | |
| relative = parsed.path or "/" | |
| if parsed.query: | |
| relative = f"{relative}?{parsed.query}" | |
| if parsed.fragment: | |
| relative = f"{relative}#{parsed.fragment}" | |
| return ( | |
| f"{base_url}{relative if relative.startswith('/') else '/' + relative}" | |
| ) | |
| return f"{base_url}/{src.lstrip('/')}" | |
| def _build_public_url(self, src: str) -> str | None: | |
| base_url = self._normalized_base_url() | |
| if not base_url or not src: | |
| return None | |
| src = str(src).strip() | |
| if src.startswith(("http://", "https://")): | |
| return src # already absolute – use as-is | |
| return f"{base_url}/{src.lstrip('/')}" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
添加把HTML上传到图床的支持
Summary by Sourcery
Add an HTML report publisher that can optionally upload generated HTML reports to an external imgbed and fall back to sending local files when needed.
New Features:
Enhancements:
Documentation:
PR Type
Enhancement, Documentation
Description
Introduce HtmlReportPublisher to upload HTML reports to imgbed
Add configuration options for imgbed upload (token, channel, enable)
Update reporting dispatch to use the new publisher with fallback
Document the imgbed upload workflow in README
Diagram Walkthrough
flowchart LR A["Generate HTML report"] --> B{"html_upload_enabled?"} B -- "Yes" --> C["Upload to imgbed"] C --> D{"Upload success?"} D -- "Yes" --> E["Send imgbed URL"] D -- "No" --> F["Send local file"] B -- "No" --> FFile Walkthrough
4 files
Integrate HtmlReportPublisher for HTML dispatchAdd HTML upload configuration methodsUse HtmlReportPublisher for HTML reportsImplement HtmlReportPublisher class1 files
Export HtmlReportPublisher1 files
Document HTML imgbed upload workflow1 files
Add html_upload_enabled and related config