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

Initial dashboard implementation #38

Merged
merged 17 commits into from
Oct 10, 2018
Merged

Initial dashboard implementation #38

merged 17 commits into from
Oct 10, 2018

Conversation

sethvincent
Copy link
Contributor

@sethvincent sethvincent commented Oct 4, 2018

This implements the dashboard based on existing mockups.

API requests were starting to get complicated enough that the use of a state management library seemed necessary. I went with unistore because it is very similar to redux while cutting down considerably on both the library size and amount of boilerplate code required compared to redux.

scoreboard-dashboard

So far I've focused on usage with tasking manager 2, but includes work to make it compatible with tasking manager 3.

@sethvincent
Copy link
Contributor Author

I'm totally stumped by some weird behavior:

When I request api('get', '/api/projects/') from the dashboard while logged out, I get the response I expect.

When I request api('get', '/api/projects/') from the dashboard while logged in, I get the same response as when I request /auth/userinfo.

It doesn't matter if this api request happens inside of an action or just called directly in a component method.

When I visit the /scoreboard/api/projects route in the browser I always get the JSON response I expect.

My best guess right now is that it has something to do with the port 3000 -> 5000 proxying that happens. I don't fully understand how this proxy works, though.

@kamicut I'm hoping you'll have an idea about what's happening here.

@sethvincent sethvincent changed the title WIP Dashboard implementation Initial dashboard implementation Oct 8, 2018
@scisco
Copy link
Contributor

scisco commented Oct 9, 2018

@sethvincent the issue you are having at when calling the api endpoints in the browser and getting json errors I think is related to this: https://github.com/developmentseed/scoreboard/blob/master/frontend/src/index.js#L9

The other issue might be related this: #43 (comment)

@sethvincent
Copy link
Contributor Author

So far I'm not seeing any difference with the service worker disabled (including making sure the service worker is unregistered).

I've worked around it by not using axios for that request, so the PR as it is now works.

@scisco
Copy link
Contributor

scisco commented Oct 10, 2018

@sethvincent great. I like removing axios all together and also i think we should separate the api frontend sooner than later.

Copy link
Contributor

@scisco scisco left a comment

Choose a reason for hiding this comment

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

This looks good to me. I think as this gets more complicated and we have a good amount of data and associated logic, we should move to properly use redux here.

@sethvincent sethvincent merged commit ec32135 into master Oct 10, 2018
@sethvincent sethvincent deleted the dashboard branch October 10, 2018 16:16
@kamicut
Copy link
Member

kamicut commented Oct 10, 2018

@sethvincent great. I like removing axios all together and also i think we should separate the api frontend sooner than later.

What do you mean by separate the api frontend?

@scisco
Copy link
Contributor

scisco commented Oct 11, 2018

@kamicut the API is currently accessed as a proxy under the same URL used for the frontend. For example if you are running the api on localhost:3000 and dashboard on localhost:5000, you access the api via locahost:5000/api instead of localhost:3000/api. This is possible with the use axios but it has a number of disadvantages and make the setup very complicated.

I think we should stop using this sort of proxying and just call the API endpoint directly.

@kamicut
Copy link
Member

kamicut commented Oct 17, 2018

I disagree with this @scisco. I think that we should take the opposite route of just rendering the pages from the server to simplify deployment. This would remove all the proxying and help with rewriting URLs related to logins.

For example, this line in https://github.com/developmentseed/scoreboard/blob/master/frontend/src/containers/Dashboard.js#L141 (and others around the app) does not make sense when hosting a public version of scoreboard. We should rewrite localhost:5000:

  • Rewrite APP_URL on the server and render the page on the server (this is the simplest logic)
  • Create a frontend REACT_APP_URL that replaces this at build time (this is the simplest fix, but requires two environment variables, APP_URL on the server and REACT_APP_URL on the frontend) cc @sethvincent

@kamicut kamicut mentioned this pull request Oct 17, 2018
@scisco
Copy link
Contributor

scisco commented Oct 18, 2018

I think we are talking about two different issues. I agree with your vision of combining the api and frontend and make it into a dynamic site (similar to a flask app) rather than a static one.

@sethvincent and I are only trying to fix the way it is currently working. We should work on your idea of combining the api and frontend into a single app separately I think.

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.

None yet

3 participants