Skip to content

refactor(workflow): Dont strip html tags in object summary#6324

Open
tanmoysrt wants to merge 2 commits into
masterfrom
chore_Workflow
Open

refactor(workflow): Dont strip html tags in object summary#6324
tanmoysrt wants to merge 2 commits into
masterfrom
chore_Workflow

Conversation

@tanmoysrt
Copy link
Copy Markdown
Member

(cherry picked from commit 16d1b6c)

@tanmoysrt tanmoysrt requested a review from ssiyad as a code owner May 4, 2026 10:13
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 4, 2026

@tanmoysrt, thanks for the contribution, but we do not accept pull requests on a master. Please close this PR and raise PR on an develop branch.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 4, 2026

Greptile Summary

This PR removes the HTML-tag sanitisation block that protected the summary field when storing Python object representations containing angle brackets (e.g. <module.Class object at 0x…>). The block was originally added because Frappe strips content resembling HTML tags from DF.Data fields on save.

  • The summary field is still typed as DF.Data; if Frappe still sanitises that field type, removing the guard will cause summaries for angle-bracket-wrapped objects to be saved as empty strings with no fallback.

Confidence Score: 3/5

Not safe to merge without confirming Frappe's Data-field HTML-stripping behaviour has changed.

There is a P1 concern: the removed code guarded against a documented Frappe framework behaviour where DF.Data fields strip angle-bracket content on save. The field type is unchanged, so if Frappe still strips HTML the summary column will silently be blank for the common Python default repr format, with no fallback.

press/workflow_engine/doctype/press_workflow_object/press_workflow_object.py — confirm whether DF.Data field type or Frappe's sanitisation behaviour has changed.

Important Files Changed

Filename Overview
press/workflow_engine/doctype/press_workflow_object/press_workflow_object.py Removes the HTML-bracket sanitisation guard for summary; the field is still DF.Data, so Frappe may silently strip angle-bracket content and leave summary empty.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["store(obj)"] --> B["summary = str(obj)"]
    B --> C{Exception?}
    C -->|Yes| D["summary = repr(type(obj))"]
    C -->|No| E["summary is set"]
    D --> E
    E --> F{len > 512?}
    F -->|Yes| G["summary = summary[:500] + '...'"]
    F -->|No| H["doc.summary = summary"]
    G --> H
    H --> I["doc.insert()"]
    I --> J{Frappe strips HTML from DF.Data?}
    J -->|Yes - angle-bracket strings| K["summary silently becomes empty string"]
    J -->|No / field type changed| L["summary saved correctly"]
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
press/workflow_engine/doctype/press_workflow_object/press_workflow_object.py:80-82
**Removed HTML-stripping guard may leave `summary` empty**

The deleted block existed specifically because Frappe silently strips content that looks like an HTML tag from `DF.Data` fields on `doc.insert()`. The `summary` field is still typed as `DF.Data` (line 47), so any object whose `str()` or `repr()` produces a string entirely enclosed in angle brackets (e.g. the default `<module.ClassName object at 0x…>`) will be saved as an empty string after Frappe's sanitisation pass — with no fallback. Before landing this change, confirm whether Frappe's behaviour for `DF.Data` fields has actually changed, or whether the field type has been migrated; otherwise the `summary` column will silently be blank for those objects.

Reviews (1): Last reviewed commit: "Merge branch 'master' into chore_Workflo..." | Re-trigger Greptile

@codecov
Copy link
Copy Markdown

codecov Bot commented May 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.65%. Comparing base (24c7b77) to head (4da1bb5).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #6324       +/-   ##
===========================================
- Coverage   82.33%   49.65%   -32.69%     
===========================================
  Files         109      936      +827     
  Lines       17374    77681    +60307     
  Branches      527      355      -172     
===========================================
+ Hits        14305    38572    +24267     
- Misses       3041    39085    +36044     
+ Partials       28       24        -4     
Flag Coverage Δ
dashboard 60.74% <ø> (-21.59%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 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.

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.

1 participant