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

feat(chat): implement apps editor on separated route (Issue #3146) #2669

Open
wants to merge 273 commits into
base: development
Choose a base branch
from

Conversation

valerydluski
Copy link
Contributor

@valerydluski valerydluski commented Nov 26, 2024

Description:

implement apps editor on separated route

Issues:

UI changes

image

Checklist:

  • the pull request name complies with Conventional Commits
  • the pull request name starts with fix(<scope>):, feat(<scope>):, feature(<scope>):, chore(<scope>):, hotfix(<scope>): or e2e(<scope>):. If contains breaking changes then the pull request name must start with fix(<scope>)!:, feat(<scope>)!:, feature(<scope>)!:, chore(<scope>)!:, hotfix(<scope>)!: or e2e(<scope>)!: where <scope> is name of affected project: chat, chat-e2e, overlay, shared, sandbox-overlay, etc.
  • the pull request name ends with (Issue #<TICKET_ID>) (comma-separated list of issues)
  • I confirm that do not share any confidential information like API keys or any other secrets and private URLs

@valerydluski valerydluski changed the title Feat/apps editor route feat(chat): apps editor route Nov 26, 2024
valerydluski and others added 21 commits November 26, 2024 14:45
@valerydluski
Copy link
Contributor Author

valerydluski commented Feb 12, 2025

/deploy-review

Environment URL: https://chat-ai-dial-chat-pr-2669.nightly-test.deltixhub.io
E2E tests status: failed

Comment on lines +64 to +68
visualizer.current = new VisualizerConnector(containerRef.current, {
domain: iframeUrl,
hostDomain: window.location.origin,
visualizerName: title,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we destroy if props have changed to avoid memory leak?

Suggested change
visualizer.current = new VisualizerConnector(containerRef.current, {
domain: iframeUrl,
hostDomain: window.location.origin,
visualizerName: title,
});
if (visualizer.current) {
visualizer.current.destroy();
}
visualizer.current = new VisualizerConnector(containerRef.current, {
domain: iframeUrl,
hostDomain: window.location.origin,
visualizerName: title,
});

@irinakartun
Copy link
Contributor

irinakartun commented Feb 13, 2025

/deploy-review

Environment URL: https://chat-ai-dial-chat-pr-2669.nightly-test.deltixhub.io
E2E tests status: failed

//code app application properties
sources: applicationData?.function?.source_folder
? ApiUtils.decodeApiUrl(applicationData.function.source_folder)
: `files/${bucket}/appdata`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Source folder should be empty as default. We have 'examples' if we don't have files in the selected directory. If we click on some example application now, we will create these files in the /appdata directory which is not supposed to be used that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated

return {
name:
applicationData?.display_name ??
getNextDefaultName(DEFAULT_APPLICATION_NAME, models ?? [], 0, true),
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to dispatch getModels on page load, otherwise we will get error on trying to create an application with default name if we open apps-editor page without navigating to it from marketplace

Снимок экрана 2025-02-13 в 15 15 42

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

app: applicationData,
runtime: pythonVersions[0],
}),
['Quick App']: getQuickAppDefaultValues({
Copy link
Contributor

@Gimir Gimir Feb 13, 2025

Choose a reason for hiding this comment

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

Quick app settings tab does not open and show infinite loader instead.
When quick app editor is open, type equals to 'QuickApps', not 'Quick App'. Same for line 130.
It's better to map this to ApplicationType enum here also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a temporary solution until Quick App has a custom editor.

@valerydluski
Copy link
Contributor Author

valerydluski commented Feb 18, 2025

/deploy-review

Environment URL: https://chat-ai-dial-chat-pr-2669.nightly-test.deltixhub.io
E2E tests status: failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request _priority_1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants