-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
fix: Info panel not showing when some apps miss infoPanel config #2627
base: alpha
Are you sure you want to change the base?
Conversation
Thanks for opening this pull request!
|
Uffizzi Ephemeral Environment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "Show panel" button doesn't appear, when having a second app in dashboard config. Before this PR, a workaround was to add an empty panel "infoPanel": [],
, but that doesn't work anymore either. Could you please verify the fix with the following configs:
1 app w/ panel:
{
"apps": [
{
"serverURL": "https://app1.example.com",
"appId": "appId1",
"masterKey": "masterKey",
"appName": "app1",
"infoPanel": [
{
"title": "User Details",
"classes": ["_User"],
"cloudCodeFunction": "user"
}
]
}
]
}
2 apps, 1 w/ panel, 1 w/o panel:
{
"apps": [
{
"serverURL": "https://app1.example.com",
"appId": "appId1",
"masterKey": "masterKey",
"appName": "app1",
"infoPanel": [
{
"title": "User Details",
"classes": ["_User"],
"cloudCodeFunction": "user"
}
]
},
{
"serverURL": "https://app2.example.com",
"appId": "appId2",
"masterKey": "masterKey",
"appName": "app2"
}
]
}
2 apps, 1 w/ panel, 1 w/ empty panel:
{
"apps": [
{
"serverURL": "https://app1.example.com",
"appId": "appId1",
"masterKey": "masterKey",
"appName": "app1",
"infoPanel": [
{
"title": "User Details",
"classes": ["_User"],
"cloudCodeFunction": "user"
}
]
},
{
"serverURL": "https://app2.example.com",
"appId": "appId2",
"masterKey": "masterKey",
"appName": "app2",
"infoPanel": []
}
]
}
2 apps, 2 w/ panel in same class:
{
"apps": [
{
"serverURL": "https://app1.example.com",
"appId": "appId1",
"masterKey": "masterKey",
"appName": "app1",
"infoPanel": [
{
"title": "User Details",
"classes": ["_User"],
"cloudCodeFunction": "user"
}
]
},
{
"serverURL": "https://app2.example.com",
"appId": "appId2",
"masterKey": "masterKey",
"appName": "app2",
"infoPanel": [
{
"title": "User Details",
"classes": ["_User"],
"cloudCodeFunction": "user"
}
]
}
]
}
2 apps, 2 w/ panel in different classes:
{
"apps": [
{
"serverURL": "https://app1.example.com",
"appId": "appId1",
"masterKey": "masterKey",
"appName": "app1",
"infoPanel": [
{
"title": "User Details",
"classes": ["_User"],
"cloudCodeFunction": "user"
}
]
},
{
"serverURL": "https://app2.example.com",
"appId": "appId2",
"masterKey": "masterKey",
"appName": "app2",
"infoPanel": [
{
"title": "Installation Details",
"classes": ["_Installation"],
"cloudCodeFunction": "installation"
}
]
}
]
}
@mtrezza, it’s working locally for me, but I'm not sure why it’s not working for you. I’ve pushed everything.
Screen.Recording.2024-11-07.005039.mp4 |
Did you try out all the configs from my previous comment? Some do not seem to work. |
Yes @mtrezza , I tried all the configurations, and they seem to be working on my end. Could you let me know which specific ones aren’t working for you? |
Uffizzi Ephemeral Environment
|
Tested it out and from the pattern I guess the issue is with how you identify an app. The tests were actually done by adding the same app twice, hence both have the same app ID, same server URL, etc. I've added the corrected configs and results below. I haven't looked at the code, but I guess you identify by app ID, hence they overwrite each other. At first I thought it was a testing mistake on my side. But then I tested the same with scripts and they work fine, even if there are 2 apps with the same app ID. I'd suggest you take a look at the script logic. 1 app w/ panel:
alpha: OK 2 apps, 1 w/ panel, 1 w/o panel:
alpha: NOK:
PR: OK 2 apps, 1 w/ panel, 1 w/ empty panel:
alpha: OK
2 apps, 2 w/ panel in same class:
alpha: NOK:
PR: NOK:
2 apps, 2 w/ panel in different classes:
alpha: NOK:
PR: NOK:
|
…into 2623 git commit --amend --author="Vardhan0604 <[email protected]>"
Hey @mtrezza, can you please review the changes? |
@vardhan0604 is the fix just adding the app name as an additional identifier? Is that also how scripts identify the app? If so, what if it's the same app name and app ID, but a different URL - would it make sense to add the URL to it? Maybe a more robust solution would be to hash the app config object, which will create a unique ID assuming that an app config will never be completely the same as the other one. But I'm not sure how reliably that would work given that objects don't have a defined key order, which may produce different hash values every time it's calculated. What do you think? |
Hey @mtrezza , regarding how scripts identify the app, I tried understanding the codebase more. Apparently, in the I also noticed that if we have two apps with the same name, we cannot see the other app here. Should we then assume that |
New Pull Request Checklist
Issue Description
Closes: #2623
Approach
TODOs before merging