Display a warning icon when a feature flag is enabled with no enabled include lists

[zackcl]

Is your feature request related to a problem? Please describe. Currently, we display a warning icon only when a feature flag is enabled with no include lists defined (with a tooltip saying "No include lists defined"). However, even when a feature flag has include lists, if none of them are enabled, the feature flag will also behave as if it is not enabled.

Describe the solution you'd like We should also show the warning icon when a feature flag is enabled with no enabled include lists, with a tooltip saying "No include lists enabled."

Describe alternatives you've considered Instead of showing different tooltip messages, would there be a good message to show in both cases for ease of implementation?

Additional context The requirement for #1838 should be updated to also check if all include lists are disabled, if any are defined. Maybe the response from the api/flags/paginated endpoint can include something like a warningState either set to "No include lists defined" or "No include lists enabled" per each flag.

[zackcl]

@danoswaltCL @bcb37 How about we add an additional field called warningState to the response from the api/flags/paginated endpoint for all flags? Would it make sense to do so?

{
    "createdAt": "2024-08-20T14:54:18.267Z",
    "updatedAt": "2024-08-20T14:55:27.839Z",
    "versionNumber": 7,
    "id": "1a27ad63-42d7-4430-bed6-abb5a4b90206",
    "name": "My Feature",
    "key": "MY_FEATURE",
    "description": "",
    "context": [
        "add"
    ],
    "tags": [],
    "status": "enabled",
    "filterMode": "excludeAll",
    "warningState": "No include lists defined",  // or "No include lists enabled", or "null?"
 }

[ppratikcr7]

@danoswaltCL @bcb37 How about we add an additional field called warningState to the response from the api/flags/paginated endpoint for all flags? Would it make sense to do so?

{
    "createdAt": "2024-08-20T14:54:18.267Z",
    "updatedAt": "2024-08-20T14:55:27.839Z",
    "versionNumber": 7,
    "id": "1a27ad63-42d7-4430-bed6-abb5a4b90206",
    "name": "My Feature",
    "key": "MY_FEATURE",
    "description": "",
    "context": [
        "add"
    ],
    "tags": [],
    "status": "enabled",
    "filterMode": "excludeAll",
    "warningState": "No include lists defined",  // or "No include lists enabled", or "null?"
 }

@zackcl We can have a simple boolean like "hasIncludeList": true/false.

[zackcl]

@ppratikcr7 The problem with hasIncludeList is that we can't show the warning icon when there are include lists, but all are disabled. The design requirement has changed, as I confirmed with Steve Ritter. Please check the upgrade-design channel for you reference.

[zackcl]

Maybe we can include a boolean variable like hasEnabledIncludeList and always show a message saying "No enabled include lists defined" in a tooltip for the warning icon? I think this will simplify the implementation. @amurphy-cl Please let me know what you think.

[danoswaltCL]

If we are not going to load all information about these flags right away and want the UI to be sufficiently dumb, then this information must come from the paginated response from the backend in some way like this.

@zackcl i agree with your point about loading things on the root page and argued unsuccessfully for that earlier on also. It would clear up a lot of things to lazy-load all major feature entities into the data store immediately on visiting any page in that feature.

It is overly optimized to load in this information a little at a time... I understand the argument is that it keeps the UI lean and dumb, which is great if it was true, but in reality, if you've spent much time developing in the UI, you get bit all the time by this. This model has led to many little oops bugs and surprises like "undefined" syntax error both here and in the experiments pages particularly when toggling between root and page views, which in turn leads to a bunch of optional operators and defensive code-checks. So the "dumb" UI has to constantly be on its toes to figure out if the information it needs about these core entities has been loaded yet. I would argue the UI is kept dumber by avoiding all of that and just loading the core details up front.

That doesn't help answer anything for mvp1, but I want to point it out.

[amurphy-cl]

@zackcl yeah this sounds good. How about just "No include lists enabled"? I don't think we need to tooltip the detail that there are also none defined.