Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #37986, #37982 - Fix UI Error when importing templates #192

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

adamlazik1
Copy link
Contributor

Importing templates through UI causes an error and the page with import results does not load.

Steps to Reproduce:

  1. Have foreman instance with the templates plugin on nightly (error originates in one of the newest commits so it is not on stream yet).

  2. Navigate to Hosts > Templates > Sync Templates

  3. Import templates from https://github.com/theforeman/community-templates.git

Actual behavior:

The page with results does not load and two popups are displayed. One saying that templates have been imported succesfully and another saying that there has been an error.

Expected behavior:

Page with results loads normally and displays templates correctly.

It appears that UI expects the error messages to be wrapped in array as opposed to being just string, and hammer (theforeman/hammer-cli-foreman-templates#15) expects an empty hash instead of nil when there are no errors.

Copy link
Member

@ofedoren ofedoren left a comment

Choose a reason for hiding this comment

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

Thanks @adamlazik1 for taking this further. I'd propose a bit different fix:

Instead of

:validation_errors => errors,

try

:validation_errors => errors || {},

@adamlazik1
Copy link
Contributor Author

@ofedoren I tried that but it didn't seem to work. It seems that UI is expecting an array somewhere down the line but I can't find it.

By the way, in regards to #190, which part was rails 7 incompatible? I am asking so that I avoid reverting it but possibly find some other solution around it.

@ofedoren
Copy link
Member

ofedoren commented Nov 6, 2024

@adamlazik1, yeah, sorry about that. It seems #190 was quite wrong, it should have been like this: https://github.com/ofedoren/foreman_templates/pull/1/files

It also reverts changes in test file to the original state.

Importing templates through UI causes an error and the page with import
results does not load.

Steps to Reproduce:

1. Have foreman instance with the templates plugin on nightly (error
originates in one of the newest commits so it is not on stream yet).

2. Navigate to Hosts > Templates > Sync Templates

3. Import templates from
https://github.com/theforeman/community-templates.git

Actual behavior:

The page with results does not load and two popups are displayed. One
saying that templates have been imported succesfully and another saying
that there has been an error.

Expected behavior:

Page with results loads normally and displays templates correctly.
@adamlazik1
Copy link
Contributor Author

Thanks @ofedoren, I applied your patch and it fixes both the UI and the hammer issue.

Copy link
Member

@ofedoren ofedoren left a comment

Choose a reason for hiding this comment

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

Thanks, @adamlazik1 !

@ofedoren ofedoren merged commit de2c8ea into theforeman:master Nov 6, 2024
22 checks passed
@adamlazik1 adamlazik1 deleted the fix-ui-error branch November 8, 2024 11:56
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.

2 participants