Event API integration complete #145

thai-truong · 2020-09-01T17:55:28Z

Still event edit page left, probably the trickiest one of them all.
Other than that not sure if there is anything else that uses event
services (if there are then we can refactor with the new event api
functions).

Just set up the user api functions file, have not done any integration
for user api.

Still event edit page left, probably the trickiest one of them all. Other than that not sure if there is anything else that uses event services (if there are then we can refactor with the new event api functions). Just set up the user api functions file, have not done any integration for user api.

netlify · 2020-09-01T17:57:15Z

Deploy preview for hkn-ucsd-portal-dev ready!

Built with commit f514af9

https://deploy-preview-145--hkn-ucsd-portal-dev.netlify.app

godwinpang · 2020-09-01T17:58:04Z

src/pages/EventDetailsPage/components/EventDetails/index.js

+  };
+
+  const eventHosts = hosts.map(host => host.id);
+  const eventType = type || 'Event';


probably better here to just type || null then handle null in tags

godwinpang · 2020-09-01T17:58:23Z

src/pages/EventDetailsPage/components/EventDetails/styles.js

+    display: 'flex',
+    flexDirection: 'column',
+    alignItems: 'center',
+    width: '536px',


I'll see what I can do with that. For now I'll worry about all the logic and functionality stuff first, then I'll move on to making it look relatively nicer once I'm done with that

netlify · 2020-09-01T17:58:41Z

Deploy preview for hknucsd-portal-dev ready!

Built with commit 6567769

https://deploy-preview-145--hknucsd-portal-dev.netlify.app

godwinpang · 2020-09-01T18:47:15Z

Yeah - I think you’re right, we can get functionality then css after demo :)

…

On Sep 1, 2020, at 11:08 AM, Thai ***@***.***> wrote: @thaigillespie commented on this pull request. In src/pages/EventDetailsPage/components/EventDetails/styles.js <#145 (comment)>: > @@ -4,33 +4,24 @@ const styles = () => ({ flexDirection: 'column', alignItems: 'center', }, - container: { - backgroundColor: 'white', + eventDetailsCard: { + display: 'flex', + flexDirection: 'column', + alignItems: 'center', + width: '536px', I'll see what I can do with that. For now I'll worry about all the logic and functionality stuff first, then I'll move on to making it look relatively nicer once I'm done with that — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#145 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AH25RGLWSJYBEWLLKXO26SDSDU2B7ANCNFSM4QSADDTA>.

godwinpang · 2020-09-02T23:46:42Z

package.json

@@ -48,7 +49,7 @@
    "build": "react-app-rewired build",
    "test": "react-app-rewired test --passWithNoTests",
    "cypress": "npx cypress run",
-    "lint": "eslint --fix './src/**/*.{js,ts,tsx}'",
+    "lint": "eslint --fix ./src/**/*.{js,ts,tsx}",


why'd you remove the '? afaik the glob pattern wasn't working without it

npm run lint didn't run for me with the ' on : (

I had to remove the ' for the command to work

i had to add ' for the command to work -_______________________-

@thaigillespie can you retest this with wsl? ' should work

Yeah I'll test it again later. ' didn't work in windows but should work in wsl.

sounds good - thanks!

godwinpang

looks great! just some small things

godwinpang · 2020-09-04T23:27:53Z

src/services/ApiUsers.ts

+  return userApi.userControllerGetMultipleUsers(queryParams);
+}
+
+export function processMultipleUsers(


not sure what this function does, could you explain why we need this? maybe I'm just not following the code

Honestly since getMultipleUsers returns a Promise with a union type, I had to make it so that the returned array is only of one of the two types from the union type, or else I couldn't use .map(). Of course, that was before changes made to what we receive as a response if we wanted to have officer names. I will have to either rewrite this in another PR. As of now, think of it as a function that extracts the users array from the returned response of type MultipleAppUserResponse.

ok - can you call it either getUsernames from response or add a TODO? ideally having an issue would be nice too :)

Yeah getUsernames is fine. I'll be making minor fixes/changes to backend tonight as well so this function hopefully will be cleaned up by tonight or tmr.

godwinpang · 2020-09-04T23:28:28Z

src/services/ApiUsers.ts

@@ -0,0 +1,97 @@
+import {


thoughts on just calling this a User service or AppUser service? that way naming lines up with backend

We use user in the frontend but appuser in the backend so I'm not sure if UserService or AppUserService is better...

I'll change it to UserService (and ApiEvents to EventService) for now

makes sense yeah - naming is hard

godwinpang · 2020-09-04T23:29:46Z

src/services/ApiUsers.ts

+  const { users } = multipleUserResponse;
+  const userKeys = Object.keys(users[0]);
+
+  if (users.length === 0) {


i think if users.length == 0 then it'll have crashed on line 46 already

godwinpang · 2020-09-04T23:32:13Z

src/pages/EventEditPage/components/EventEditForm/form_chip_list_input.js

@@ -1,40 +0,0 @@
-import React from 'react';


i think there's value in keeping form_chip..._input.js files around for historical purposes - they'd look pretty nice in our storybook as well :) what do you think?

That's fine. I just removed them bc I thought we weren't gonna use them anymore but hey no harm in keeping them. I didn't use them in my integration code but maybe someone else in the future will have a need for it.

godwinpang · 2020-09-04T23:35:55Z

src/constants/eventStatus.ts

+  COMPLETE: 'complete',
+};
+
+export default EVENT_STATUS;


i think for these enums we'd be able to leverage code gen

see this for an example

* Got a lot of event api integration done, but not all. Still event edit page left, probably the trickiest one of them all. Other than that not sure if there is anything else that uses event services (if there are then we can refactor with the new event api functions). Just set up the user api functions file, have not done any integration for user api. * Added autocompletion component * Finished with integration for event edit page. * Added changes to address review of PR #145. * Removed duplicate config to turn off import/prefer-default-export. * Just for deploy preview.

* Got a lot of event api integration done, but not all. Still event edit page left, probably the trickiest one of them all. Other than that not sure if there is anything else that uses event services (if there are then we can refactor with the new event api functions). Just set up the user api functions file, have not done any integration for user api. * Added autocompletion component * Finished with integration for event edit page. * Added changes to address review of PR #145. * Removed duplicate config to turn off import/prefer-default-export. * Just for deploy preview. Rebase modal_factor's commits onto master

thai-truong added enhancement New feature or request feature Build in something new labels Sep 1, 2020

thai-truong requested a review from a team September 1, 2020 17:55

thai-truong self-assigned this Sep 1, 2020

Merge branch 'dev' into frontendIntegration

4bb549d

godwinpang requested changes Sep 1, 2020

View reviewed changes

Added autocompletion component

7faa611

godwinpang reviewed Sep 2, 2020

View reviewed changes

Finished with integration for event edit page.

956f124

godwinpang requested changes Sep 4, 2020

View reviewed changes

Added changes to address review of PR #145.

0765273

godwinpang previously approved these changes Sep 5, 2020

View reviewed changes

thai-truong changed the title ~~Got a lot of event api integration done, but not all (WIP)~~ Event API integration complete Sep 5, 2020

Merge branch 'dev' into frontendIntegration

5a9ded8

thai-truong dismissed godwinpang’s stale review via 5a9ded8 September 5, 2020 05:34

Thai added 2 commits September 4, 2020 22:39

Removed duplicate config to turn off import/prefer-default-export.

01e50ec

Just for deploy preview.

6567769

thai-truong merged commit bac6f32 into dev Sep 5, 2020

thai-truong linked an issue Sep 5, 2020 that may be closed by this pull request

Frontend openapi integration to start using backend API w/o fetch API #142

Closed

godwinpang deleted the frontendIntegration branch September 5, 2020 21:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Event API integration complete #145

Event API integration complete #145

thai-truong commented Sep 1, 2020

netlify bot commented Sep 1, 2020

godwinpang Sep 1, 2020

godwinpang Sep 1, 2020

thai-truong Sep 1, 2020

netlify bot commented Sep 1, 2020 •

edited

Loading

godwinpang commented Sep 1, 2020 via email

godwinpang Sep 2, 2020

thai-truong Sep 2, 2020

thai-truong Sep 2, 2020

godwinpang Sep 2, 2020

godwinpang Sep 4, 2020

thai-truong Sep 4, 2020

godwinpang Sep 4, 2020

godwinpang left a comment

godwinpang Sep 4, 2020

thai-truong Sep 5, 2020

godwinpang Sep 5, 2020

thai-truong Sep 5, 2020

godwinpang Sep 4, 2020

thai-truong Sep 5, 2020

thai-truong Sep 5, 2020

godwinpang Sep 5, 2020

godwinpang Sep 4, 2020

godwinpang Sep 4, 2020

thai-truong Sep 5, 2020

godwinpang Sep 4, 2020

Event API integration complete #145

Event API integration complete #145

Conversation

thai-truong commented Sep 1, 2020

netlify bot commented Sep 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Sep 1, 2020 • edited Loading

godwinpang commented Sep 1, 2020 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

godwinpang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Sep 1, 2020 •

edited

Loading