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

Operator Api | Introduce product validations #203

Merged
merged 6 commits into from Mar 31, 2023

Conversation

tom-ioki
Copy link
Contributor

This PR will introduce the ProductValidation model and a custom endpoint product_validations that allows fetching all validations for a specific product.

@tom-ioki tom-ioki force-pushed the introduce-product-validations branch from 6d9c8df to eb1138c Compare March 15, 2023 15:32
Copy link

@jai-ioki jai-ioki left a comment

Choose a reason for hiding this comment

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

Tested this locally. Looks good. I just have a small remark.


attribute :affected_items,
on: :read,
type: :array,

Choose a reason for hiding this comment

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

This is probably intentional but it startled me for a moment:

When there are no affected items like for not_live the API returns null

  {
        "id": "not_live",
        "severity": "danger",
        "title": "Product NOT live",
        "description": "This product is in \"planned\" mode. If you operate a service, make sure to set this product to \"live\" mode, as non live products skip safety checks and critical errors might not get detected.",
        "affected_items": null,
        "type": "product_bulletin"
      }

When reading the attribute definition above I would expect a conversion, however its nil

{:type=>"product_bulletin",
 :id=>"not_live",
 :severity=>"danger",
 :title=>"Produkt NICHT Live",
 :description=>
  "Dieses Produkt befindet sich im Modus \"planned\". Wenn Sie einen Dienst betreiben, stellen Sie sicher, dass dieses Produkt in den \"Live\"-Modus versetzt wird, da Produkte, die nicht Live sind, Sicherheitsprüfungen überspringen und kritische Fehler möglicherweise nicht erkannt werden.",
 :affected_items=>nil}

Copy link
Contributor Author

@tom-ioki tom-ioki Mar 31, 2023

Choose a reason for hiding this comment

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

You mean, you'd expect an empty array for affected_items?

Copy link

@jai-ioki jai-ioki Mar 31, 2023

Choose a reason for hiding this comment

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

An empty array, yes

{
:type=>"product_bulletin", 
:id=>"not_live", 
:severity=>"danger", 
:title=>"Produkt NICHT Live", 
:description=> "Dieses Produkt befindet sich im Modus \"planned\". Wenn Sie einen Dienst betreiben, stellen Sie sicher, dass dieses Produkt in den \"Live\"-Modus versetzt wird, da Produkte, die nicht Live sind, Sicherheitsprüfungen überspringen und kritische Fehler möglicherweise nicht erkannt werden.", 
:affected_items=>[]
}

But I'm guessing that attributes are optional by default. If that is the convention then that's fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point!
The issue can be found in the ProductBulletin class in triebwerk. The default value for affected_items is nil 😬 . I will make a follow up PR in triebwerk. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's change it to an empty array, that's more consistent.

@tom-ioki tom-ioki merged commit 152a389 into main Mar 31, 2023
@tom-ioki tom-ioki deleted the introduce-product-validations branch March 31, 2023 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants