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

UI updates & parse script #50

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Conversation

alaaltoros
Copy link

@alaaltoros alaaltoros commented Apr 6, 2020

PR related to issue #21 (Front-End UI for MVP)

It includes the following:

Basic UI
Login/logout with google
Location data parse script
I Will be updating it as soon as other parts are ready.

P.S. The login/logout functionality requires google client id (check the README.md) and the upload button will output the parsed data in the console.

@alaaltoros alaaltoros mentioned this pull request Apr 6, 2020
4 tasks
@ainsleys
Copy link
Contributor

ainsleys commented Apr 6, 2020

Hey @alaaltoros can you add a description to this PR explaining what it is?

@alaaltoros
Copy link
Author

PR related to issue #21 (Front-End UI for MVP)

It includes the following:

  • Basic UI
  • Login/logout with google
  • Location data parse script

I Will be updating it as soon as other parts are ready.

P.S. The login/logout functionality requires google client id (check the README.md) and the upload button will output the parsed data in the console.

@alaaltoros
Copy link
Author

Added report and file upload basic functionality

@ainsleys ainsleys requested a review from lacabra April 8, 2020 17:57
Copy link
Contributor

@lacabra lacabra left a comment

Choose a reason for hiding this comment

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

Dear @alaaltoros, thanks for your contribution in building a UI for this project. I have done an initial review of this PR, but I cannot get it to work, meaning: I have installed the package deps, configured the client, and launched it, and it displays on the browser. The connection with Google OAuth seems fine, but after trying to log in, I get an error message Login failed. I believe this may be due mostly to gaps in documentation. Kindly review the items I've highlighted in my review, and ping me with a mention when these are addressed, so that I can do a more in-depth review.

Thanks again!

UIcode/README.md Show resolved Hide resolved
UIcode/README.md Outdated
touch .env.development

# set API url i.e. http://localhost:4080
echo "REACT_APP_API_URL=http://localhost:4080" >> .env.development
Copy link
Contributor

Choose a reason for hiding this comment

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

I am confused by this instruction to configure the API on port 4080, but the server is started below on port 3000. Can you please document this better, explaining the difference between the two ports if they are indeed different ports?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

This is the URL of the backend API, I added more description to this file.

client/scripts/last_14d_visits.js Show resolved Hide resolved
UIcode/server.js Outdated Show resolved Hide resolved
@alaaltoros
Copy link
Author

alaaltoros commented Apr 9, 2020

Hello @lacabra,
I updated the README.md file and the directory structure.
Let me know if you can get the client to run or if you have any other comments or doubts.
Please note that the backend is included in a different repository and a different pull request (I added a URL in the README.md file)

@alaaltoros
Copy link
Author

Added the Results page with formatted data. However, I could not test with a real response from findMatches endpoint of Enclave; I received no matches and could not upload report new results because of the CORS error.

@cmalfesi
Copy link
Contributor

@alaaltoros I have tested your last changes and in general, it looks good. Regarding the issue that returns CORS error, I have done some tests and I see that the problem occurs when there are 2 consecutive calls to the Enclave endpoints. If I execute one only it works. I think the problem is related to the promise or await functions.

@cmalfesi
Copy link
Contributor

@alaaltoros
I have an improvement for the frontend to get better behavior with consecutive calls to the enclave endpoints. We need to separate addPersonalData and findMatch in different async functions. This solves the CORS problem. I'll send you the required changes.

@assafmo
Copy link
Member

assafmo commented Apr 20, 2020

@alaaltoros can you resolve the conflicts? Also waiting for @cmalfesi CORS fixes in order to merge this.

@alaaltoros
Copy link
Author

@assafmo conflicts resolved!

@assafmo
Copy link
Member

assafmo commented Apr 20, 2020

Still seeing conflicts in UIcode/client/package-lock.json and UIcode/client/yarn.lock.
You can remove the one you don't use and rebuild the one you use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants