Help Requested: Content view error message - #22498

Does anyone have experience/knowledge with content views/components? Specifically anything related to error messages? I’m working on this bug: Bug #22498: Improve help and error messages when adding CVs to a CCV - Katello - Foreman which doesn’t provide the desired error message. You get the error message when trying to add a duplicate component to a composite content view via hammer: hammer content-view add-version --id 6 --content-view Zoo --content-view-version-id 4, for example.

Problem is, at the component level, we get the correct error, as seen here: https://github.com/Katello/katello/blob/master/app/models/katello/content_view_component.rb#L45, but somehow, the content view is overwriting the error message with a more generic message, “Validation failed: Content view components is invalid”.

@Jonathon_Turel and I have done some digging and I’m having a hard time finding where this is overwritten. Does anyone have experience with the issue?

Here’s a snippet which demonstrates the behavior specifically:

  view1 = create(:katello_content_view)                                                                                                                                  
  version1 = create(:katello_content_view_version, :content_view => view1)                                                                                               
                                                                                                                                                                         
  composite = katello_content_views(:composite_view)                                                                                                                     
  component = ContentViewComponent.create(:composite_content_view => composite,                                                                                          
                               :content_view_version => version1)                                                                                                        
                                                                                                                                                                         
  version2 = create(:katello_content_view_version, :content_view => view1)                                                                                               
  composite.update_attributes!(component_ids: [version1.id, version2.id])

composite.errors.full_messages will not have the full validation errors that arise from the new ContentViewComponent that’s getting created as part of the update. Maybe we need to change the way we are updating the content view somehow?

I spoke with @Michael_Johnson about this today and while I don’t have a solution, I have a theory about what is happening.

When rails is running update_attributes! on the content view, it is updating the dependent models, like content_view_components. It does this, runs the validation on this model, it fails, and by default it shows “associated model is invalid” instead of “My very specific error message from associated model”.

It seems like this stackoverflow article has run into a similar issue but I’m not a huge fan of the accepted solution.

The other part that is confusing is we don’t use validates_associated, but I think when you update associations, it runs automatically? I would have to look into it more.

This is of course just a theory and I could be off base, but it does look like that is the default error message ("foo is invalid) for validates_associated so I would point the finger at Rails. Hope this helps! :wink:

Yes, validates_associated does in fact run automatically. I was able to fix this bug by using validates_associated though, giving it specific parameters. This should provide more clarity around the errors.

The issue was that content view components model had the correct error, but since content views is the one that made the call, its errors trumped those of content view components. With validates_associated, we were able to combine the two.

Definitely consider using this for other similar class structures when we need something from the lower-level class that we aren’t getting.