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

Update task creation form #743

Merged
merged 10 commits into from
Aug 24, 2023
12 changes: 6 additions & 6 deletions packages/dashboard-e2e/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@ browser.overwriteCommand(
```

```ts
// this will fail depending on the context on which `getLoopOption` is called.
const getLoopOption = async () => {
return $$('[role=option]').find(async (elem) => await elem.getText() === 'Loop');
// this will fail depending on the context on which `getPatrolOption` is called.
const getPatrolOption = async () => {
return $$('[role=option]').find(async (elem) => await elem.getText() === 'Patrol');
};

await browser.waitUntil(async () => !!(await getLoopOption)); // ok
await console.log(await getLoopOption()); // error
await browser.waitUntil(async () => !!(await getPatrolOption)); // ok
await console.log(await getPatrolOption()); // error
```

Possible reason is because async selectors chaining relies on a "finalizer" or some kind of context to resolve the promises. As a result, when running `getLoopOption` without a wdio await, the chaining does not work. But this is all speculation, the inner workings of async selector chainings are very complex.
Possible reason is because async selectors chaining relies on a "finalizer" or some kind of context to resolve the promises. As a result, when running `getPatrolOption` without a wdio await, the chaining does not work. But this is all speculation, the inner workings of async selector chainings are very complex.
12 changes: 6 additions & 6 deletions packages/dashboard-e2e/tests/ui-interactions/submit-task.test.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
import { getAppBar } from '../utils';

describe('submit task', () => {
it('can submit loop task', async () => {
it('can submit patrol task', async () => {
const appBar = await getAppBar();
await (await appBar.$('button[aria-label="Tasks"]')).click();
await (await appBar.$('button[aria-label="new task"]')).click();
await (await $('#task-type')).click();
const getLoopOption = async () => {
const getPatrolOption = async () => {
const options = await $$('[role=option]');
for (const opt of options) {
const text = await opt.getText();
if (text === 'Loop') {
if (text === 'Patrol') {
return opt;
}
}
return null;
};
await browser.waitUntil(async () => !!(await getLoopOption()));
const loopOption = (await getLoopOption())!;
await loopOption.click();
await browser.waitUntil(async () => !!(await getPatrolOption()));
const patrolOption = (await getPatrolOption())!;
await patrolOption.click();

await (await $('#place-input')).setValue('coe');

Expand Down
34 changes: 16 additions & 18 deletions packages/dashboard/src/components/appbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import {
AppBarTab,
CreateTaskForm,
CreateTaskFormProps,
getPlaces,
getNamedPlaces,
HeaderBar,
LogoButton,
NavigationBar,
Expand Down Expand Up @@ -167,8 +167,10 @@ export const AppBar = React.memo(({ extraToolbarItems }: AppBarProps): React.Rea
const [brandingIconPath, setBrandingIconPath] = React.useState<string>('');
const [settingsAnchor, setSettingsAnchor] = React.useState<HTMLElement | null>(null);
const [openCreateTaskForm, setOpenCreateTaskForm] = React.useState(false);
const [placeNames, setPlaceNames] = React.useState<string[]>([]);
const [workcells, setWorkcells] = React.useState<string[]>();
const [waypointNames, setWaypointNames] = React.useState<string[]>([]);
const [cleaningZoneNames, setCleaningZoneNames] = React.useState<string[]>([]);
const [pickupPointNames, setPickupPointNames] = React.useState<string[]>([]);
const [dropoffPointNames, setDropoffPointNames] = React.useState<string[]>([]);
const [favoritesTasks, setFavoritesTasks] = React.useState<TaskFavorite[]>([]);
const [refreshTaskAppCount, setRefreshTaskAppCount] = React.useState(0);
const [username, setUsername] = React.useState<string | null>(null);
Expand Down Expand Up @@ -214,23 +216,20 @@ export const AppBar = React.memo(({ extraToolbarItems }: AppBarProps): React.Rea
})();
}, [logoResourcesContext, safeAsync, curTheme]);

React.useEffect(() => {
if (!resourceManager?.dispensers) {
return;
}
setWorkcells(Object.keys(resourceManager.dispensers.dispensers));
}, [resourceManager]);

React.useEffect(() => {
if (!rmf) {
return;
}

const subs: Subscription[] = [];
subs.push(
rmf.buildingMapObs.subscribe((map) =>
setPlaceNames(getPlaces(map).map((p) => p.vertex.name)),
),
rmf.buildingMapObs.subscribe((map) => {
const namedPlaces = getNamedPlaces(map);
setPickupPointNames(namedPlaces.pickupPoints.map((w) => w.vertex.name));
setDropoffPointNames(namedPlaces.dropoffPoints.map((w) => w.vertex.name));
setCleaningZoneNames(namedPlaces.cleaningZones.map((w) => w.vertex.name));
setWaypointNames(namedPlaces.places.map((w) => w.vertex.name));
}),
);
subs.push(
AppEvents.refreshAlertCount.subscribe((_) => {
Expand Down Expand Up @@ -578,11 +577,10 @@ export const AppBar = React.memo(({ extraToolbarItems }: AppBarProps): React.Rea
{openCreateTaskForm && (
<CreateTaskForm
user={username ? username : 'unknown user'}
cleaningZones={placeNames}
loopWaypoints={placeNames}
deliveryWaypoints={placeNames}
dispensers={workcells}
ingestors={workcells}
cleaningZones={cleaningZoneNames}
patrolWaypoints={waypointNames}
pickupPoints={pickupPointNames}
dropoffPoints={dropoffPointNames}
favoritesTasks={favoritesTasks}
open={openCreateTaskForm}
onClose={() => setOpenCreateTaskForm(false)}
Expand Down
46 changes: 46 additions & 0 deletions packages/react-components/lib/place.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,21 @@
import type { BuildingMap, GraphNode } from 'api-client';

const DEFAULT_PICKUP_POINT_PARAM_NAME = 'pickup_dispenser';
const DEFAULT_DROPOFF_POINT_PARAM_NAME = 'dropoff_ingestor';
const DEFAULT_CLEANING_ZONE_PARAM_NAME = 'is_cleaning_zone';

export interface Place {
level: string;
vertex: GraphNode;
}

export interface NamedPlaces {
places: Place[];
pickupPoints: Place[];
dropoffPoints: Place[];
cleaningZones: Place[];
}

export function getPlaces(buildingMap: BuildingMap): Place[] {
const places: Place[] = [];
for (const level of buildingMap.levels) {
Expand All @@ -18,3 +29,38 @@ export function getPlaces(buildingMap: BuildingMap): Place[] {
}
return places;
}

export function getNamedPlaces(buildingMap: BuildingMap): NamedPlaces {
const places = new Map<string, Place>();
const pickupPoints = new Map<string, Place>();
const dropoffPoints = new Map<string, Place>();
const cleaningZones = new Map<string, Place>();

for (const level of buildingMap.levels) {
for (const graphs of level.nav_graphs) {
for (const vertex of graphs.vertices) {
if (vertex.name) {
const place: Place = { level: level.name, vertex };
for (const p of vertex.params) {
if (p.name === DEFAULT_PICKUP_POINT_PARAM_NAME) {
pickupPoints.set(vertex.name, place);
}
if (p.name === DEFAULT_DROPOFF_POINT_PARAM_NAME) {
dropoffPoints.set(vertex.name, place);
}
if (p.name === DEFAULT_CLEANING_ZONE_PARAM_NAME) {
cleaningZones.set(vertex.name, place);
}
}
places.set(vertex.name, place);
}
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Maybe we can use destructuring and switch here

Suggested change
for (const level of buildingMap.levels) {
for (const graphs of level.nav_graphs) {
for (const vertex of graphs.vertices) {
if (vertex.name) {
const place: Place = { level: level.name, vertex };
for (const p of vertex.params) {
if (p.name === DEFAULT_PICKUP_POINT_PARAM_NAME) {
pickupPoints.set(vertex.name, place);
}
if (p.name === DEFAULT_DROPOFF_POINT_PARAM_NAME) {
dropoffPoints.set(vertex.name, place);
}
if (p.name === DEFAULT_CLEANING_ZONE_PARAM_NAME) {
cleaningZones.set(vertex.name, place);
}
}
places.set(vertex.name, place);
}
}
}
}
for (const level of buildingMap.levels) {
for (const graphs of level.nav_graphs) {
for (const vertex of graphs.vertices) {
const { name, params } = vertex;
if (!name) {
continue;
}
const place: Place = { level: level.name, vertex };
for (const p of params) {
switch (p.name) {
case DEFAULT_PICKUP_POINT_PARAM_NAME:
pickupPoints.set(name, place);
break;
case DEFAULT_DROPOFF_POINT_PARAM_NAME:
dropoffPoints.set(name, place);
break;
case DEFAULT_CLEANING_ZONE_PARAM_NAME:
cleaningZones.set(name, place);
break;
}
}
places.set(name, place);
}
}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

9837319

I've just refactored the if case! Thanks for catching that, @Angatupyry!

Although the repeated if cases look ugly, I think I'll keep it that way since we have no guarantees that a single waypoint can't have multiple of these params (e.g. all 3), also I believe switch statements should be reserved for enums or simple number and chars, I don't think we should be comparing strings that way.

I'll go ahead and merge this now! Thanks!

return {
places: Array.from(places.values()),
pickupPoints: Array.from(pickupPoints.values()),
dropoffPoints: Array.from(dropoffPoints.values()),
cleaningZones: Array.from(cleaningZones.values()),
};
}
7 changes: 3 additions & 4 deletions packages/react-components/lib/tasks/create-task.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ export const CreateTask: Story<CreateTaskFormProps> = (args) => {
CreateTask.args = {
submitTasks: async () => new Promise((res) => setTimeout(res, 500)),
cleaningZones: ['test_zone_0', 'test_zone_1'],
loopWaypoints: ['test_waypoint_0', 'test_waypoint_1'],
deliveryWaypoints: ['test_waypoint_0', 'test_waypoint_1'],
dispensers: ['test_dispenser_0', 'test_dispenser_1'],
ingestors: ['test_ingestor_0', 'test_ingestor_1'],
patrolWaypoints: ['test_waypoint_0', 'test_waypoint_1'],
pickupPoints: ['test_waypoint_0'],
dropoffPoints: ['test_waypoint_1'],
};
Loading
Loading