rename an api call to make room for another one
swap swr for xservice
add test
edit the paths used to read a template and create a workspace because the API has changed since that code was written and they were broken - I did this in the fastest way possible to unblock myself without bloating this PR; I'll change the create workspace flow to use XState when I work on workspace CRUD
editrenderWithAuth to work with parameterized routes

presleyp added9 commits

May 4, 2022 17:11

Change name of existing workspace call

386a2d7

Add new api call (has handler already)

c4c4b3c

WorkspacesPage -> WorkspacePage

7e3db42

starting to replace swr

f46c970

Add other api calls

74a5d1d

Fix api call

b0e4b5a

Replace swr with xstate

495aba6

Format

14be28e

Test - wip

fc98515

presleyp self-assigned this

May 4, 2022

Copy link

codecovbot commentedMay 4, 2022•
edited
Loading

Codecov Report

Merging#1294 (78f1708) intomain (f911c8a) willincrease coverage by0.34%.
The diff coverage is71.42%.

@@            Coverage Diff             @@##             main    #1294      +/-   ##==========================================+ Coverage   66.08%   66.43%   +0.34%==========================================  Files         281      282       +1       Lines       18424    18466      +42       Branches      220      231      +11     ==========================================+ Hits        12176    12268      +92+ Misses       4984     4940      -44+ Partials     1264     1258       -6

Flag	Coverage Δ
unittest-go-macos-latest	`53.63% <ø> (+0.14%)`	⬆️
unittest-go-postgres-	`65.00% <ø> (+0.06%)`	⬆️
unittest-go-ubuntu-latest	`55.98% <ø> (+0.01%)`	⬆️
unittest-go-windows-2022	`51.88% <ø> (-0.02%)`	⬇️
unittest-js	`73.14% <71.42%> (+1.53%)`	⬆️

Impacted Files	Coverage Δ
site/src/AppRouter.tsx	`0.00% <ø> (ø)`
...anizationPage/TemplatePage/CreateWorkspacePage.tsx	`0.00% <0.00%> (ø)`
...ges/OrganizationPage/TemplatePage/TemplatePage.tsx	`0.00% <0.00%> (ø)`
site/src/xServices/workspace/workspaceXService.ts	`59.09% <59.09%> (ø)`
site/src/api/index.ts	`71.83% <90.90%> (+4.08%)`	⬆️
site/src/pages/WorkspacePage/WorkspacePage.tsx	`91.66% <91.66%> (ø)`
site/src/forms/CreateWorkspaceForm.tsx	`91.30% <100.00%> (+1.83%)`	⬆️
site/src/testHelpers/index.tsx	`100.00% <100.00%> (+5.55%)`	⬆️
site/src/xServices/StateContext.tsx	`92.85% <100.00%> (+0.54%)`	⬆️
site/src/xServices/terminal/terminalXService.ts	`68.42% <100.00%> (ø)`
... and11 more

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last updatef911c8a...78f1708. Read thecomment docs.

presleyp added4 commits

May 4, 2022 23:33

Fix route in template page

c0ad87b

Fix endpoint in create workspace

60532a8

Fix tests

cfac4c2

Lint

78f1708

presleyp marked this pull request as ready for review

May 5, 2022 04:40

presleyp requested a review froma team as acode owner

May 5, 2022 04:40

kylecarbs approved these changes

May 5, 2022

View reviewed changes

Copy link

Member

kylecarbs left a comment

Choose a reason for hiding this comment

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

LGTM!

site/src/forms/CreateWorkspaceForm.tsx

		@@ -15,13 +15,19 @@ export interface CreateWorkspaceForm {
		template: Template
		onSubmit: (request: CreateWorkspaceRequest) => Promise<Workspace>
		onCancel: () => void
		organization_id: string

Copy link

Member

Choose a reason for hiding this comment

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

Should this beorganizationID to match othercamelCase fields in this type?

Copy link

ContributorAuthor

Choose a reason for hiding this comment

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

I left it snakecased because it gets piped directly out of and into api calls, but I agree the inconsistency isn't good. I guess I'll ask about this at FE V, we have some other spots like this.

site/src/pages/WorkspacePage/WorkspacePage.tsxShow resolvedHide resolved

site/src/pages/WorkspacePage/WorkspacePage.tsx

		const workspaceId = firstOrItem(workspaceQueryParam, null)

		const xServices = useContext(XServiceContext)
		const [workspaceState, workspaceSend] = useActor(xServices.workspaceXService)

Copy link

Member

Choose a reason for hiding this comment

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

If I understand correctly, this uses a global XState service. WoulduseMachine be more appropriate here since the machine should only be active for the lifecycle of this page?

Copy link

ContributorAuthor

Choose a reason for hiding this comment

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

I was thinking it would be active for longer but maybe that's not actually desired since it's for a specific workspace. I can make it local for now anduseMachine and we can hoist it later if needed.

Copy link

Member

kylecarbsMay 5, 2022

Choose a reason for hiding this comment

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

Neato neato

Copy link

ContributorAuthor

presleypMay 5, 2022•
edited
Loading

Choose a reason for hiding this comment

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

@kylecarbs Actually I think we'll want it to outlive the component because we're using separate pages for things like the create/edit form and the parts of the timeline. Does that sound right?

presleyp changed the title~~Workspace api/presleyp~~fix: create and read workspace page

May 5, 2022

code-asher approved these changes

May 5, 2022

View reviewed changes

Copy link

Member

code-asher left a comment

Choose a reason for hiding this comment

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

Looks great!

I wonder if we could tweak the workspace endpoint in the future to include more info so we can avoid three calls on the FE but I might be overthinking it 😂

presleyp merged commita2be7c0 intomain

May 6, 2022

presleyp deleted the workspace-api/presleyp branch

May 6, 2022 14:02

misskniss added the V2 BETA label

May 15, 2022

misskniss added this to theV2 Beta milestone

May 15, 2022

kylecarbs pushed a commit that referenced this pull request

Jun 10, 2022

fix: create and read workspace page (#1294)

67a0c74

* Change name of existing workspace call* Add new api call (has handler already)* WorkspacesPage -> WorkspacePage* starting to replace swr* Add other api calls* Fix api call* Replace swr with xstate* Format* Test - wip* Fix route in template page* Fix endpoint in create workspace* Fix tests* Lint

Labels

None yet

4 participants

Movatterモバイル変換

fix: create and read workspace page#1294

fix: create and read workspace page#1294

Uh oh!

Conversation

presleyp commentedMay 4, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

codecovbot commentedMay 4, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kylecarbs left a comment

Choose a reason for hiding this comment

Uh oh!

kylecarbsMay 5, 2022

Choose a reason for hiding this comment

Uh oh!

presleypMay 5, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kylecarbsMay 5, 2022

Choose a reason for hiding this comment

Uh oh!

presleypMay 5, 2022

Choose a reason for hiding this comment

Uh oh!

kylecarbsMay 5, 2022

Choose a reason for hiding this comment

Uh oh!

presleypMay 5, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

code-asher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

presleyp commentedMay 4, 2022•
edited
Loading

codecovbot commentedMay 4, 2022•
edited
Loading

presleypMay 5, 2022•
edited
Loading