search

freeCodeCamp / meeting-for-good

A meeting coordination app for your team
https://meeting.freecodecamp.org
BSD 3-Clause "New" or "Revised" License
338 stars 113 forks source link

Add common propTypes for Events #398

Open jrogatis opened 7 years ago

jrogatis commented 7 years ago

We use a common library of proptypes. Add events type check and change the object at each one of the files that use this prop.

caskuplich commented 7 years ago

@jrogatis I'd like to help with this issue. How can I start?

gURLmeetsCode commented 7 years ago

@jrogatis I would like to help as well. Please me know if there is anything I can do.

jrogatis commented 7 years ago

sorry about the delayed answer! Yes all help will be appreciated! @gURLmeetsCode and @caskuplich ! Its simple. look at the code and find the complex prototypes that repeat along the other component classes and add a file inside at the commom prototypes dir and try to export then there. Any help that you needed please contact me !

caskuplich commented 7 years ago

@jrogatis Let me see if I get it: I found at least 3 files that could use the common propTypes isEvent and isCurUser from the file client/util/commonPropTypes.js:

But I haven't identified patterns of complex propTypes that could be reused by components. Maybe you can help me find these propTypes.

jrogatis commented 7 years ago

@caskuplich curUser: PropTypes.shape({ _id: PropTypes.string, // Unique user id name: PropTypes.string, // User name avatar: PropTypes.string, // URL to image representing user(?) }).isRequired,

is one of than is used all over the app.

caskuplich commented 7 years ago

@jrogatis Unfortunately I can't run the project on my local machine. After Google redirects to the app, the following error appears:

Error
    at /<...>/meeting-for-good/node_modules/passport-google-oauth20/lib/strategy.js:95:21
    at passBackControl (/<...>/meeting-for-good/node_modules/oauth/lib/oauth2.js:132:9)
    at IncomingMessage.<anonymous> (/<...>/meeting-for-good/node_modules/oauth/lib/oauth2.js:157:7)
    at emitNone (events.js:110:20)
    at IncomingMessage.emit (events.js:207:7)
    at IncomingMessage.wrappedEmit [as emit] (/<...>/meeting-for-good/node_modules/opbeat/lib/instrumentation/http-shared.js:116:29)
    at endReadableNT (_stream_readable.js:1045:12)
    at instrumented (/<...>/meeting-for-good/node_modules/opbeat/lib/instrumentation/index.js:102:27)
    at _combinedTickCallback (internal/process/next_tick.js:138:11)
    at process._tickDomainCallback (internal/process/next_tick.js:218:9)
    at process.fallback (/<...>/meeting-for-good/node_modules/opbeat/lib/instrumentation/async-hooks.js:514:17)

Node version: v8.3.0 MongoDB version: v3.4.7 OS: Trisquel 7 (Ubuntu 14.04)

Can you help me with this issue?

jrogatis commented 7 years ago

@caskuplich Hey there. Sure! Do you set the .env file properly with the correct api keys ?

caskuplich commented 7 years ago

@jrogatis I just didn't set the FACEBOOK_KEY, FACEBOOK_SECRET, AWS_ACCESS_KEY_ID, AWS_SECRET_KEY, and EMAIL_FROM.

Is there any problem to set the URL restrictions and redirect_url to http://localhost:8080 and http://localhost:8080/api/auth/google/callback, respectively on the Google API Console when I create a new OAuth API client_id? I enabled the Calendar API as stated on the README file and also created an API Key for Google Analytics.

jrogatis commented 7 years ago

You need to set this call back properly at the google api console. http://localhost:8080/api/auth/google/callback at the javascript authorized URLs at restrictions area. @caskuplich

caskuplich commented 7 years ago

This is my Google Developers console settings:

console

This is a OAuth Client ID credential.

It seems that the problem is on the meeting-for-good code on the return. For some reason the code returned from Google causes the error I mentioned above. The URL of the return comes with a code query string, like this: http://localhost:8080/api/auth/google/callback?code=<code with 46 chars>. This is right, isn't it?

jrogatis commented 7 years ago

do you set the Oauth consentiment screen ? and enable the API ? @caskuplich ?

caskuplich commented 7 years ago

@jrogatis, yes. I set my email address and the Product Name as "Meeting for Good Dev". The other fields are optional.

Can I sign-in the application with the same email address I set in the Consent screen or should I enter with a different Google Account?

caskuplich commented 7 years ago

I'm suspicious that it's a version problem. I think that there is some breaking change in passport.js or other related package. @jrogatis, can you make a new clone of the repo run on your local machine?

jrogatis commented 7 years ago

@caskuplich I will try today.

jrogatis commented 7 years ago

@caskuplich its running perfectly.

caskuplich commented 7 years ago

@jrogatis, thank you so much. I'll try to find what is wrong with my local machine setup.

jrogatis commented 7 years ago

what . errors you got at the back ? @caskuplich ?

caskuplich commented 7 years ago

The same errors I posted above, @jrogatis. What makes this hard to solve is that there isn't a good error message, unfortunately.

caskuplich commented 7 years ago

@jrogatis Yeah! Now I can login! I had to enable the Google+ API to make this work. So, my Google API console dashboard has the Google Calendar API, Google+ API and the Analytics API enabled. Thank you again @jrogatis. Now I can work on this issue on my local machine. :smile:

jrogatis commented 7 years ago

great @caskuplich !

caskuplich commented 7 years ago

@jrogatis , I DRYed the NotificationBar component (client/components/NotificationBar/NotificationBar.js) with no problems. But I tried to change the NavBar component (client/components/NavBar/NavBar.js) and it issues a warning on the web console saying that _id and avatar are required when the user isn't logged in. The current version doesn't issue any warning regarding propTypes. Can you help me with this?

I'd also want to DRY the client/pages/NewEvent/index.js file, and I noted that it is already issuing a warning on the console about the user propType right now (current version). I think that we should fix this warning before DRYing this file. What do you think?

jrogatis commented 7 years ago

@caskuplich can you give me your branch url ? so i can see the code and help with ?

caskuplich commented 7 years ago

@jrogatis Here is my branch URL: https://github.com/caskuplich/meeting-for-good/tree/fix/refactor-proptypes

I think the NotificationBar can be merged with no problems, but the NavBar issues propTypes warnings saying that _id, name, and avatar from curUser are undefined even when the user is logged in.

I also noticed that the client/pages/NewEvent/index.js (current version, I didn't modify it yet) issues a warning saying curUser propType is undefined just the first time we click the + button at the bottom right corner of the screen. When we click the next times it doesn't issue any warning.

jrogatis commented 6 years ago

i will adress this ASAP @caskuplich for some reason I miss your last comment