meeting-for-good icon indicating copy to clipboard operation
meeting-for-good copied to clipboard

Add common propTypes for Events

Open jrogatis opened this issue 7 years ago • 24 comments

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.

jrogatis avatar Jun 20 '17 20:06 jrogatis

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

caskuplich avatar Aug 15 '17 18:08 caskuplich

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

gURLmeetsCode avatar Aug 19 '17 18:08 gURLmeetsCode

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 !

jrogatis avatar Sep 09 '17 14:09 jrogatis

@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:

  • client/components/NavBar/NavBar.js
  • client/components/NotificationBar/NotificationBar.js
  • client/pages/NewEvent/index.js

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

caskuplich avatar Sep 09 '17 20:09 caskuplich

@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.

jrogatis avatar Sep 10 '17 04:09 jrogatis

@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?

caskuplich avatar Sep 12 '17 02:09 caskuplich

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

jrogatis avatar Sep 12 '17 11:09 jrogatis

@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.

caskuplich avatar Sep 12 '17 18:09 caskuplich

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

jrogatis avatar Sep 13 '17 12:09 jrogatis

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?

caskuplich avatar Sep 13 '17 18:09 caskuplich

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

jrogatis avatar Sep 13 '17 18:09 jrogatis

@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 avatar Sep 13 '17 21:09 caskuplich

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?

caskuplich avatar Sep 16 '17 19:09 caskuplich

@caskuplich I will try today.

jrogatis avatar Sep 17 '17 20:09 jrogatis

@caskuplich its running perfectly.

jrogatis avatar Sep 17 '17 21:09 jrogatis

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

caskuplich avatar Sep 19 '17 15:09 caskuplich

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

jrogatis avatar Sep 19 '17 15:09 jrogatis

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

caskuplich avatar Sep 19 '17 15:09 caskuplich

@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:

caskuplich avatar Sep 19 '17 17:09 caskuplich

great @caskuplich !

jrogatis avatar Sep 19 '17 18:09 jrogatis

@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?

caskuplich avatar Sep 20 '17 01:09 caskuplich

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

jrogatis avatar Sep 20 '17 15:09 jrogatis

@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.

caskuplich avatar Sep 20 '17 19:09 caskuplich

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

jrogatis avatar Nov 27 '17 04:11 jrogatis