Skip to content

Fixes #39463 - show error toasts correct#11045

Draft
sbernhard wants to merge 1 commit into
theforeman:developfrom
ATIX-AG:fix_39463
Draft

Fixes #39463 - show error toasts correct#11045
sbernhard wants to merge 1 commit into
theforeman:developfrom
ATIX-AG:fix_39463

Conversation

@sbernhard

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions github-actions Bot added the UI label Jun 25, 2026
@sbernhard
sbernhard marked this pull request as draft June 25, 2026 11:40
@sbernhard
sbernhard marked this pull request as ready for review July 3, 2026 15:45

@chris1984 chris1984 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks fine to me, I asked Claude to look and it's saying it's present in two other areas, did you want to fix them in this pr as well?

  • BulkBuildHostModal.js:42 — { ...response.data.error, key: HOST_BUILD_KEY } will crash identically when response is undefined (e.g. network error). It destructures { response } but doesn't guard it.
    • BulkReassignHostgroupModal.js:130 — ...response.data.error with the old response parameter signature (not destructured from the error object at all), so it'll crash on both network errors and any error shape where the callback receives { response, message }.

@sbernhard
sbernhard force-pushed the fix_39463 branch 2 times, most recently from cd2e639 to d6c5394 Compare July 9, 2026 20:16
@sbernhard
sbernhard marked this pull request as draft July 9, 2026 20:16
@sbernhard

Copy link
Copy Markdown
Contributor Author

@chris1984 Thanks. Yes, this make sense.
Added these cases and even more; restructured the PR; added tests ... but not yet tested.

@chris1984 chris1984 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for fixing the other things I suggested. Will let @jeremylenz have a final look to make sure I didn't miss anything

@jeremylenz

Copy link
Copy Markdown
Contributor

LGTM, @sbernhard please mark as ready for review if this is ready to merge.

@Lukshio

Lukshio commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Little bit late to the party. But could you please provide more information about the issue? If I understand it correctly. Due to some parsing issues or foreman limitations (max len. 60 chars.) you are using custom addToast instead of APIActions build in successToast and errorToast.

And this new code should unify the parsing for all of the places?

We opened issue which should solve the parsing problems directly in APIActions unfortunately we are not there yet :/
https://projects.theforeman.org/issues/39296

@Lukshio Lukshio self-assigned this Jul 17, 2026

@Lukshio Lukshio 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.

I've just made a quick look at the code and found some things to clarify.
Also could you please update the PR title to be more accurate with what is changing?

Comment thread webpack/assets/javascripts/react_app/components/HostsIndex/BulkActions/helpers.js Outdated
Co-authored-by: Codex <codex@openai.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants