Dev: Add BIT backend heroku url #267

mtreacy002 · 2021-07-06T16:03:50Z

Description

This PR supports partial integration of BIT and MS backend on Heroku remote servers by deploying BIT frontend that uses BIT and MS backend heroku servers as REST endpoints.

Fixes #266

Type of Change:

Code

Code/Quality Assurance Only

New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

Tested on Maya Treacy's BIT web deployed on fork repo github.io by creating a new user and login after verifying email.
BIT frontend : https://mtreacy002.github.io/bridge-in-tech-web/
BIT backend: https://bridge-in-tech-backend.herokuapp.com/
MS backend: https://ms-backend-bit.herokuapp.com/

Checklist:

My PR follows the style guidelines of this project
I have performed a self-review of my own code or materials
I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
I have made corresponding changes to the documentation

Code/Quality Assurance Only

My changes generate no new warnings
My PR currently breaks something (fix or feature that would cause existing functionality to not work as expected)
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Additional Notes

Ideally, we use dotenv that comes with react-scripts package and use .env file instead of constantly commenting in/out lines in config.js. However, I only know how to do this for local .env but not sure how to do it on gh-pages. So, this comment-in/out lines in config.js hopefully temporary until we can figure out how to do this efficiently using gh-pages environment.
Following note no. 1 above, once the pr is approved, the config.js file needs to be updated (by commenting out line 4 and uncomment line 3)

mtreacy002 · 2021-07-06T16:16:02Z

@Rahulm2310 , I'm not sure why test is failing. Can you please give me a hint here? Thanks beforehand.

epicadk · 2021-07-06T17:27:21Z

src/register/Register.jsx

@@ -36,6 +36,7 @@ export default function Register() {
                headers: {
                    "Accept": "application/json",
                    "Content-Type": "application/json",
+                    "Access-Control-Allow-Origin": {CORS_ORIGIN}


Does the browser not automatically add these? I have no clue about front end I always thought the browser would auto generate these in a "pre-flight" request.

@epicadk , yeah, unfortunately it did not. Before I added this, the api calls received error stating No Access-Control-Allow-Origin header is present... and it was cleared after I added this.

mtreacy002 · 2021-07-08T14:14:01Z

@isabelcosta , now that we have 2 approvals, should I refactor CORS origin in config.js to be the anitab-org gh-pages url instead of my fork repo before you merge it to develop branch? I used my gh-pages url so that reviewer can test the deployed app. I mentioned this as additional notes in the issue description above 😉.

isabelcosta · 2021-07-09T23:06:41Z

@mtreacy002 yes you can refactor :) This seems to make sense ;) I trust your decision here, just let me know when I should take a look again and help with review.

add heroku specific branch on fork repo Refactor homepage url to fork repo for test remove extra backslash add CORS ORIGIN for remote BIT web on Login Add CORS ORIGIN headers on other existing features refactor to staging remote urls remove travis yml refactor config to include both upstream and fork BIT web url revert dotenv since gh action use different method for secret refactor config update config refactor to upstream BIT web org

mtreacy002 · 2021-07-10T00:50:51Z

@mtreacy002 yes you can refactor :) This seems to make sense ;) I trust your decision here, just let me know when I should take a look again and help with review.

Done, @isabelcosta . I've refactored it to the actual BIT web on anitab-org.github.io. Hopefully this will work so that BIT finally can have the remote application 🙏.
PS: we won't be able to see it until this PR is merge, since the branch to deploy has also now refactored to develop only (because we don't have bit-heroku-url on the upstream repo).
If we want to see the website being deployed before pr is merged (for testing), a new branch bit-heroku-url need to be created at the upstream repo (we need you for this as I don't have authority on the upstream repo 😆. But I personally don't think it's necessary since we've tested it on my fork repo.

isabelcosta · 2021-07-10T10:47:24Z

@mtreacy002 the tests are failing. do you know why? Can you fix it before we merge?

mtreacy002 · 2021-07-17T03:14:04Z

@mtreacy002 the tests are failing. do you know why? Can you fix it before we merge?

will try look into this, @isabelcosta . Hope @Rahulm2310 and @jalajcodes can also help me point out the possible cause here 🙏

mtreacy002 · 2021-07-17T04:16:28Z

@isabelcosta , @Rahulm2310 and @jalajcodes . I noticed that when I changed the BASE_API to localhost:5000 and CORS_ORIGIN to localhost:3000, the tests passed, but then when I switched it back to the remote urls (BASE_API https://bridge-in-tech-backend.herokuapp.com/ and CORS_ORIGIN https://anitab-org.github.io), the tests failed. I'm not sure why it's reacting like that. Can you please give me suggestions on how to fix this? PS: we need the default url to be set to the remote urls for the deployed BIT web to work remotely.
Screenshots

tests passed with BASE_API AND CORR_ORIGIN set to localhosts

on remote urls

jalajcodes · 2021-07-17T11:25:40Z

@mtreacy002 I am not sure why its passing with localhost as origin.

But from what I can see in this file, we are searching for Name : label twice and the second test is failing with the error Found multiple elements with the text of: Name :. Looks like its caching the elements. Can you try importing cleanup from @testing-library/react (check this) and see if tests are passing.

mtreacy002 · 2021-07-17T12:40:50Z

@jalajcodes , thank you for your tips. However, I'm still confused. The doc you referred to above stated that if we're using jest as testing framework (which we are), the cleanup should be automatically done. Although, I tried to use the afterEach(cleanup) anyway but for some reasons the tests still failed (2 out of 5 though, maybe I use the afterEach(cleanup) wrong, hehe). Can you please tell me where I did wrong? Thanks.

I added afterEach(cleanup) on login.test.jsx line 28

Here's the gist with the error log

But, adding that line (afterEach(cleanup)) didn't affect the tests if the urls are changed to local (tests still passed), just as before.

jalajcodes · 2021-07-18T03:02:54Z

@mtreacy002 I'll take a look at it later today

jalajcodes · 2021-07-20T15:12:44Z

@mtreacy002 Sorry for replying late, actually the problem is with the version of msw package, upgrading it to latest version should make the tests pass.

mtreacy002 · 2021-07-21T02:08:02Z

@mtreacy002 Sorry for replying late, actually the problem is with the version of msw package, upgrading it to latest version should make the tests pass.

Thanks for the tips, @jalajcodes . I'll give it a try and update you on this 😉

mtreacy002 requested review from isabelcosta, vj-codes, epicadk and Rahulm2310 July 6, 2021 16:04

epicadk reviewed Jul 6, 2021

View reviewed changes

epicadk previously approved these changes Jul 6, 2021

View reviewed changes

Rahulm2310 previously approved these changes Jul 7, 2021

View reviewed changes

mtreacy002 added Category: Coding Changes to code base or refactored code that doesn't fix a bug. Status: Ready to Merge Work has been tested and needs a final review and merge from a repo maintainer. Type: Maintenance Repository maintenance. labels Jul 8, 2021

mtreacy002 dismissed stale reviews from Rahulm2310 and epicadk via df06420 July 10, 2021 00:48

mtreacy002 force-pushed the bit-heroku-url branch from dcbc0c9 to df06420 Compare July 10, 2021 00:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dev: Add BIT backend heroku url #267

Dev: Add BIT backend heroku url #267

mtreacy002 commented Jul 6, 2021 •

edited

Loading

mtreacy002 commented Jul 6, 2021

epicadk Jul 6, 2021 •

edited

Loading

mtreacy002 Jul 7, 2021

mtreacy002 commented Jul 8, 2021

isabelcosta commented Jul 9, 2021

mtreacy002 commented Jul 10, 2021 •

edited

Loading

isabelcosta commented Jul 10, 2021

mtreacy002 commented Jul 17, 2021

mtreacy002 commented Jul 17, 2021

jalajcodes commented Jul 17, 2021

mtreacy002 commented Jul 17, 2021 •

edited

Loading

jalajcodes commented Jul 18, 2021

jalajcodes commented Jul 20, 2021

mtreacy002 commented Jul 21, 2021

Dev: Add BIT backend heroku url #267

Are you sure you want to change the base?

Dev: Add BIT backend heroku url #267

Conversation

mtreacy002 commented Jul 6, 2021 • edited Loading

Description

Type of Change:

How Has This Been Tested?

Checklist:

mtreacy002 commented Jul 6, 2021

epicadk Jul 6, 2021 • edited Loading

Choose a reason for hiding this comment

mtreacy002 Jul 7, 2021

Choose a reason for hiding this comment

mtreacy002 commented Jul 8, 2021

isabelcosta commented Jul 9, 2021

mtreacy002 commented Jul 10, 2021 • edited Loading

isabelcosta commented Jul 10, 2021

mtreacy002 commented Jul 17, 2021

mtreacy002 commented Jul 17, 2021

jalajcodes commented Jul 17, 2021

mtreacy002 commented Jul 17, 2021 • edited Loading

jalajcodes commented Jul 18, 2021

jalajcodes commented Jul 20, 2021

mtreacy002 commented Jul 21, 2021

mtreacy002 commented Jul 6, 2021 •

edited

Loading

epicadk Jul 6, 2021 •

edited

Loading

mtreacy002 commented Jul 10, 2021 •

edited

Loading

mtreacy002 commented Jul 17, 2021 •

edited

Loading