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

Workshop app - Phase 1 #831

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Conversation

sparkplug0025
Copy link
Contributor

@sparkplug0025 sparkplug0025 commented Sep 4, 2024

wip...

image

@sparkplug0025
Copy link
Contributor Author

@brandonfancher & @Velua can you please review the UI structure, the form hooks etc. To make sure it is aligned with the other frontend apps patterns?

@James-Mart package wip, need to sync with you!

}

#[action]
fn storeSys(path: String, contentType: String, content: Hex<Vec<u8>>) {
Copy link
Member

Choose a reason for hiding this comment

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

storeSys (and therefore the WebContentTable) should be in the query service alongside serveSys.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when I read query my first instinct was that it is a read only service (I thought the same when I saw the example named r-<package>). If we are adding the storesys it would go against a query or a read only service. So what is the definition of the query service on the package?

Copy link
Collaborator

Choose a reason for hiding this comment

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

A query service's primary job is to handle HTTP requests. In this mode it is read-only. The data is stored in the query service because it is only used for the HTTP interface. It's essentially static content associated with the service.

Copy link
Member

Choose a reason for hiding this comment

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

I think I might prefer calling it an "http-server" or something. Query / RPC service don't seem as readily understandable.

services/user/Workshop/service/src/lib.rs Outdated Show resolved Hide resolved
/// Holds metadata for a registered app
#[table(name = "AppMetadataTable", index = 0)]
#[derive(Debug, Clone, Fracpack, ToSchema, Serialize, Deserialize, SimpleObject)]
pub struct AppMetadata {
Copy link
Member

Choose a reason for hiding this comment

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

These weren't in the github issue (until right now because I just added them), but could you also add

  • tags: List<String> - Devs can add up to 3 tags that can further be used to categorize the application
  • created: TimePointSec - This can be set with getting currentBlock().time from the Transact service. Tracks the unix time in seconds that the app was registered.

services/user/Workshop/query/src/lib.rs Outdated Show resolved Hide resolved
@brandonfancher
Copy link
Contributor

This looks good. I see that some of the UI and account selection controls were borrowed from webmail/chainmail. There is a newer in-progress (will be merged soon) branch of that that actually queries available accounts and changes the account with supervisor from the dropdown (vs. using the mocked out accounts list fixture.) That can be found in the bf/chainmail-enhancements branch. That also has a dark mode toggle. I recommend updating the account switcher from that if it's relevant to this app and bringing in the dark mode toggle.

UI code wise, everything looks great!

@James-Mart
Copy link
Member

Dark mode, pretty please!

@James-Mart James-Mart changed the title Workshop App Workshop app - Phase 1 Sep 4, 2024
@James-Mart James-Mart added the System app Related to system services and their apps/plugins label Sep 4, 2024
@James-Mart James-Mart modified the milestone: R2 Sep 4, 2024
@sparkplug0025
Copy link
Contributor Author

@swatanabe can you please help me troubleshoot why I'm receiving the error below when running cargo package from the Workshop root folder?

error: r-workshop is not a dependency of workshop

@swatanabe
Copy link
Collaborator

@swatanabe can you please help me troubleshoot why I'm receiving the error below when running cargo package from the Workshop root folder?

error: r-workshop is not a dependency of workshop

It's triggered by server = "r-workshop"

When trying to resolve "r-workshop" we look at the workspace members and the immediate dependencies of "workshop". "r-workshop" isn't in either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
System app Related to system services and their apps/plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants