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

Populate submitted values in site details #1043

Merged
merged 2 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/api/src/sites/sites.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,12 @@ export class Site {
@Column({ nullable: true, select: false, type: 'character varying' })
contactInformation?: string | null;

hasHobo: boolean;
hasHobo?: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

Was this change needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, since it was a required property in Site TS expected sites.service.ts::findeOne to return this property as well. The spread operator ...site tricked TS into thinking that hasHobo is included in the response, even though it was actually not. After removing the spread operator and explicitly defining the properties, TS throws an error that hasHobo is missing. I had to make this property optional to overcome this error and since it's not always defined, it makes sense to be optional.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, the alternative would be to actually have it defined no? Since it can only be true or false?

Copy link
Collaborator Author

@K-Markopoulos K-Markopoulos Oct 25, 2024

Choose a reason for hiding this comment

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

Yes, we can calculate the value and have it defined. I didn't add it in the first place, because I didn't want to change the structure of the response (the initial response didn't have hasHobo property) and it seems like it requires another query to calculate it.

const hasHoboDataSet = await hasHoboDataSubQuery(this.sourceRepository);


collectionData?: CollectionDataDto;

maskedSpotterApiToken?: string;

@Expose()
get applied(): boolean {
return !!this.siteApplication?.permitRequirements;
Expand Down
42 changes: 34 additions & 8 deletions packages/api/src/sites/sites.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,13 +207,18 @@ export class SitesService {
}

async findOne(id: number): Promise<Site> {
const site = await getSite(id, this.sitesRepository, [
'region',
'admins',
'historicalMonthlyMean',
'siteApplication',
'sketchFab',
]);
const site = await getSite(
id,
this.sitesRepository,
[
'region',
'admins',
'historicalMonthlyMean',
'siteApplication',
'sketchFab',
],
true,
);

// Typeorm returns undefined instead of [] for
// OneToMany relations, so we fix it to match OpenAPI specs:
Expand All @@ -227,8 +232,29 @@ export class SitesService {
this.latestDataRepository,
);

const maskedSpotterApiToken = site.spotterApiToken
? `****${site.spotterApiToken?.slice(-4)}`
: undefined;

return {
...site,
id: site.id,
name: site.name,
sensorId: site.sensorId,
polygon: site.polygon,
nearestNOAALocation: site.nearestNOAALocation,
depth: site.depth,
iframe: site.iframe,
status: site.status,
maxMonthlyMean: site.maxMonthlyMean,
timezone: site.timezone,
display: site.display,
createdAt: site.createdAt,
updatedAt: site.updatedAt,
region: site.region,
admins: site.admins,
siteApplication: site.siteApplication,
sketchFab: site.sketchFab,
maskedSpotterApiToken,
surveys,
historicalMonthlyMean,
videoStream,
Expand Down
10 changes: 10 additions & 0 deletions packages/api/src/sites/sites.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export const siteTests = () => {
});
expect(rsp.body.historicalMonthlyMean).toBeDefined();
expect(rsp.body.historicalMonthlyMean.length).toBe(12);
expect(rsp.body.maskedSpotterApiToken).toBeUndefined();
});

it('GET /:id/daily_data', async () => {
Expand All @@ -135,12 +136,21 @@ export const siteTests = () => {
.put(`/sites/${siteId}`)
.send({
name: updatedSiteName,
spotterApiToken: '123456789',
});

expect(rsp.status).toBe(200);
expect(rsp.body).toMatchObject({ name: updatedSiteName });
});

it('GET /:id retrieve masked spotterApiToken', async () => {
const rsp = await request(app.getHttpServer()).get(`/sites/${siteId}`);

expect(rsp.status).toBe(200);
expect(rsp.body.maskedSpotterApiToken).toBe('****6789');
expect(rsp.body.spotterApiToken).toBeUndefined();
});

it.each([
['https://aqualink.org', 200],
['http:aqualink.org/protocol', 400],
Expand Down
8 changes: 7 additions & 1 deletion packages/website/src/common/Forms/TextField.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { ChangeEvent } from 'react';
import React, { ChangeEvent, FocusEvent } from 'react';
import {
withStyles,
WithStyles,
Expand All @@ -20,6 +20,8 @@ const CustomTextfield = ({
size,
disabled,
onChange,
onBlur,
onFocus,
classes,
select = false,
}: CustomTextfieldProps) => {
Expand All @@ -33,6 +35,8 @@ const CustomTextfield = ({
type={isNumeric ? 'number' : 'text'}
value={formField.value}
onChange={onChange}
onBlur={onBlur}
onFocus={onFocus}
label={label}
placeholder={placeholder}
name={name}
Expand Down Expand Up @@ -65,6 +69,8 @@ interface CustomTextfieldIncomingProps {
onChange: (
event: ChangeEvent<HTMLInputElement | HTMLTextAreaElement>,
) => void;
onBlur?: (event: ChangeEvent<HTMLInputElement | HTMLTextAreaElement>) => void;
onFocus?: (event: FocusEvent<HTMLInputElement | HTMLTextAreaElement>) => void;
select?: boolean;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ const EditForm = ({
const { latitude: draftLatitude, longitude: draftLongitude } =
draftSite?.coordinates || {};

const [editToken, setEditToken] = React.useState(false);
const [useDefaultToken, setUseDefaultToken] = React.useState(false);
const [editToken, setEditToken] = React.useState(
!!site.maskedSpotterApiToken,
);

const [editContactInfo, setEditContactInfo] = React.useState(false);

Expand Down Expand Up @@ -88,11 +89,13 @@ const EditForm = ({
);

const [siteSpotterApiToken, setSiteSpotterApiToken] = useFormField<string>(
'',
site.maskedSpotterApiToken ?? '',
['maxLength'],
);

const [status, setStatus] = useFormField<string>('', []);
const [apiTokenChanged, setApiTokenChanged] = React.useState(false);

const [status, setStatus] = useFormField<string>(site.status, []);

const [display, setDisplay] = useFormField<boolean>(site.display, []);

Expand All @@ -110,18 +113,29 @@ const EditForm = ({
'maxLength',
]);

const onApiTokenBlur = () => {
if (siteSpotterApiToken.value === '' && site.maskedSpotterApiToken) {
setSiteSpotterApiToken(site.maskedSpotterApiToken);
setApiTokenChanged(false);
}
};
const onApiTokenFocus = () => {
if (!apiTokenChanged) {
setSiteSpotterApiToken('');
}
};

const formSubmit = (event: FormEvent<HTMLFormElement>) => {
if (
siteName.value &&
siteDepth.value &&
siteLatitude.value &&
siteLongitude.value
) {
const insertedTokenValue = siteSpotterApiToken.value
const insertedTokenValue = apiTokenChanged
? siteSpotterApiToken.value
: undefined;
const tokenValue = useDefaultToken ? null : insertedTokenValue;
const spotterApiToken = editToken ? tokenValue : undefined;
const spotterApiToken = editToken ? insertedTokenValue : null;
// fields need to be undefined in order not be affected by the update.
// siteSensorId.value here can be <empty string> which our api does not accept
const sensorId = siteSensorId.value || undefined;
Expand Down Expand Up @@ -178,6 +192,7 @@ const EditForm = ({
break;
case 'spotterApiToken':
setSiteSpotterApiToken(newValue);
setApiTokenChanged(true);
break;
case 'status':
setStatus(newValue);
Expand Down Expand Up @@ -266,35 +281,17 @@ const EditForm = ({
/>
</Grid>
{editToken && (
<>
<Grid item sm={8} xs={8}>
<TextField
disabled={useDefaultToken}
formField={siteSpotterApiToken}
label="Spotter API token"
placeholder="Spotter API token"
name="spotterApiToken"
onChange={onFieldChange}
/>
</Grid>
<Grid
item
xs={4}
style={{ display: 'flex', justifyContent: 'flex-end' }}
>
<FormControlLabel
labelPlacement="start"
control={
<Checkbox
color="primary"
checked={useDefaultToken}
onChange={() => setUseDefaultToken(!useDefaultToken)}
/>
}
label="Use default token"
/>
</Grid>
</>
<Grid item xs={12}>
<TextField
formField={siteSpotterApiToken}
label="Spotter API token"
placeholder="Spotter API token"
name="spotterApiToken"
onChange={onFieldChange}
onBlur={onApiTokenBlur}
onFocus={onApiTokenFocus}
/>
</Grid>
)}
<Grid item xs={12}>
<Alert className={classes.infoAlert} icon={false} severity="info">
Expand Down
1 change: 1 addition & 0 deletions packages/website/src/store/Sites/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ export interface Site {
sketchFab?: SiteSketchFab;
display: boolean;
contactInformation?: string;
maskedSpotterApiToken?: string;
iframe?: string | null;
}

Expand Down
Loading