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: add 'Open in Explore' link for each apps on studio #11402

Merged
merged 5 commits into from
Dec 9, 2024

Conversation

kurokobo
Copy link
Contributor

@kurokobo kurokobo commented Dec 5, 2024

Summary

This PR adds an "Open in Explore" link to the menu of each app in the app list in Studio, and the menu under Publish button in Studio.
Clicking this link will open the target app in Explore in a new tab.
Also, to prevent the items in the pop-up menu from containing line break, I will slightly widen its width.

To achieve this, following changes are made by this PR:

  • API
    • Update /console/api/installed-apps to allow querying installed apps by app_id
  • Web
    • Add Open in Explore menu for each apps in the app list on studio
    • Add 'Open in Explore' in the publish menu on studio

Closes #11401

Screenshots

Before: After:

image
image
image

image
image
image
image

Checklist

Important

Please review the checklist below before submitting your pull request.

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. ☕️ typescript Pull request that update TypeScript code. 💪 enhancement New feature or request labels Dec 5, 2024
@kurokobo kurokobo marked this pull request as draft December 7, 2024 16:09
@kurokobo kurokobo marked this pull request as ready for review December 7, 2024 16:33
@kurokobo
Copy link
Contributor Author

kurokobo commented Dec 7, 2024

I have updated the PR and marked it as Ready for Review.

When "Open in Explore" is clicked, it fetches /console/api/installed-apps?app_id=${app_id} to get the corresponding Installed App ID, and then opens /explore/installed/${installed_app_id}.

I also considered fetching the list of all Installed Apps when opening the Studio to find the corresponding ID, but since the creation of the Installed App is executed asynchronously and triggered by events, it seems that we cannot guarantee the existence of the Installed App immediately after it is created. Therefore, I implemented this to fetch the ID when clicking the menu instead.

I would appreciate your review. Thanks in advance!

Copy link
Member

@crazywoola crazywoola left a comment

Choose a reason for hiding this comment

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

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 9, 2024
@laipz8200 laipz8200 merged commit 230fa32 into langgenius:main Dec 9, 2024
6 checks passed
@laipz8200
Copy link
Member

laipz8200 commented Dec 9, 2024

Hi @kurokobo! Thank you for your contribution! It would be great if you could add a new get method in the InstalledAppApi and pass the app ID in the URL path, rather than using the InstalledAppsListApi. Would you mind making this improvement in a separate PR? 😊

@kurokobo kurokobo deleted the open_in_explore branch December 9, 2024 16:26
@kurokobo
Copy link
Contributor Author

kurokobo commented Dec 9, 2024

@laipz8200
Hi, thanks for the review and merge!

I’m thinking about working on improvements to the API, but I’d like your opinion on the design of the endpoints.

Assumptions:

  • For a specific app, its AppID and InstalledAppID are different.
  • Currently, the App model only has the AppID, and doesn’t have the InstalledAppID.
  • The InstalledApp model currently holds both the InstalledAppID and AppID.

Required endpoints to enable Open in Explore feature:

  • An endpoint that allows looking up the InstalledAppID using the AppID, instead of looking up the AppID using the installedAppID

Questions about your suggestions:

  • Even if we can GET /installed-app/<InstalledAppID>, it does not allow us to look up the InstalledAppID using the AppID.
  • If we allow GET /installed-app/<AppID>, we can look up the InstalledAppID using the AppID, but I feel uneasy passing a different model's ID as part of the URL under /installed-app.
  • Wouldn’t it be more appropriate to add /apps/<AppID>/installed-app under /apps instead?

Please share your thoughts, thanks!

@laipz8200
Copy link
Member

@kurokobo
Thank you for your reminder. I believe what you said makes more sense; let's keep the current implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💪 enhancement New feature or request lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files. ☕️ typescript Pull request that update TypeScript code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "Open in Explore" link in the menu for each apps in Studio
3 participants