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

Mijn-1316/electrische laadpalen #1117

Merged
merged 10 commits into from
Feb 6, 2024
Merged

Conversation

OscarBakker
Copy link
Contributor

No description provided.

@OscarBakker OscarBakker force-pushed the feat1316/electrische-laadpalen branch from 7ca8394 to 7037405 Compare January 17, 2024 09:54
@OscarBakker OscarBakker marked this pull request as ready for review January 17, 2024 10:11
@OscarBakker OscarBakker force-pushed the feat1316/electrische-laadpalen branch 2 times, most recently from 77cdf40 to 13a79c1 Compare January 23, 2024 09:03
Transform Laadpaal detail response. Add detailpanel oplaadpunten. Do a detail loopup after composing the urls content with the next urls content. Add transformers, nextUrls capabilities. Composed detail lookup still not working. Electrische laadpalen toegevoed aan mijn buurtkaart. Add charge-point.svg icon.
Different name for beschikbaar and snel beschikbaar
Remove unnecessary config in buurt.ts
Refactor source api request.
Refactor multiple request interceptors, next request logic.
@OscarBakker OscarBakker force-pushed the feat1316/electrische-laadpalen branch from 13a79c1 to f530f03 Compare January 24, 2024 11:13
@OscarBakker OscarBakker changed the title Feat1316/electrische laadpalen Mijn-1316/electrische laadpalen Jan 24, 2024
@@ -62,6 +62,7 @@ export interface DataRequestConfig extends AxiosRequestConfig {
cancelTimeout?: number;
postponeFetch?: boolean;
urls?: Record<string, string>;
nextUrls?: string[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Kun je deze weghalen? In source-api-request doen we niks met nextUrls

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screenshot 2024-02-05 at 09 57 26 actually needed

src/server/helpers/interceptors.ts Outdated Show resolved Hide resolved
src/server/services/buurt/next-request.ts Outdated Show resolved Hide resolved
src/server/services/buurt/datasets.ts Outdated Show resolved Hide resolved
src/server/services/buurt/datasets.ts Outdated Show resolved Hide resolved
src/server/services/buurt/datasets.ts Outdated Show resolved Hide resolved
src/universal/config/myarea-datasets.ts Show resolved Hide resolved
src/server/services/buurt/datasets.ts Show resolved Hide resolved
src/server/services/buurt/datasets.ts Show resolved Hide resolved
src/server/services/buurt/buurt.ts Show resolved Hide resolved
@@ -426,24 +425,10 @@ export const datasetEndpoints: Record<
'https://map.data.amsterdam.nl/maps/oplaadpunten?SERVICE=WFS&VERSION=2.0.0&REQUEST=GetFeature&TYPENAMES=ms:snel_beschikbaar&OUTPUTFORMAT=geojson&SRSNAME=urn:ogc:def:crs:EPSG::4326',
],
},
disabled: IS_AP,
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah sorry Oscar, zo kunnen we het alleen op test bekijken. Beter is disabled: IS_PRODUCTION

Copy link
Contributor

@timvanoostrom timvanoostrom left a comment

Choose a reason for hiding this comment

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

Bijna! :-)

requestConfig.request = config.requestConfig.request;
}

const response: any = await requestData(requestConfig, requestID);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const response: any = await requestData(requestConfig, requestID);
const response = await requestData(requestConfig, requestID);

any is niet nodig,
Screenshot 2024-02-05 at 13 45 31

requestConfig.url,
'https://map.data.amsterdam.nl/maps/oplaadpunten?SERVICE=WFS&VERSION=2.0.0&REQUEST=GetFeature&TYPENAMES=ms:snel_beschikbaar&OUTPUTFORMAT=geojson&SRSNAME=urn:ogc:def:crs:EPSG::4326',
];
const requests = urls?.map((url) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const requests = urls?.map((url) => {
const requests = urls.map((url) => {

Optional chaining is hier niet vereist aangezien urls altijd bestaat.

Comment on lines 520 to 539
let responses: any;

try {
responses = await Promise.all(requests);
} catch (error) {
return error;
}

responses = responses.filter((res: any) => res.data);

// If transformDetail is called return the first response for the detail view
if (responses[0].data._embedded) return responses[0];

// Combine all responses into one for the list view
if (responses.length > 1) {
responses[0].data = responses.slice(1).reduce((acc: any, response: any) => {
return acc.concat(response.data);
}, responses[0].data);
}
return responses[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

Je kent de type hier dus any is niet nodig. Je hoeft de Promise.all ook niet te wrappen in try/catch, dat gebeurt al in source-api-request.

En ipv reduce, kun je niet beter iets doen als:

const responses = await Promise.all(requests);
responses[0].data = responses.map((response)=>response.data).flat();
return responses[0];

@OscarBakker OscarBakker force-pushed the feat1316/electrische-laadpalen branch from c4cd305 to 3976d07 Compare February 5, 2024 13:36
@OscarBakker OscarBakker merged commit 878d036 into main Feb 6, 2024
3 of 7 checks passed
@OscarBakker OscarBakker deleted the feat1316/electrische-laadpalen branch February 6, 2024 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants