-
Notifications
You must be signed in to change notification settings - Fork 8
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
fx: change the logic for fetching activity logs #816
Conversation
src/apis/userActivityLoggerAxios.js
Outdated
@@ -0,0 +1,28 @@ | |||
import axios from "axios"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parsing error: require() of ES Module /usr/local/node_modules/eslint/node_modules/eslint-scope/lib/definition.js from /usr/local/node_modules/babel-eslint/lib/require-from-eslint.js not supported.
Instead change the require of definition.js in /usr/local/node_modules/babel-eslint/lib/require-from-eslint.js to a dynamic import() which is available in all CommonJS modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, @khalifan-kfan, instead of creating a new file and creating duplicate code, try to do something I did in the same axios file here
Hey Khalifan, let us merge into |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @khalifan-kfan, maybe a few things to modify
src/apis/userActivityLoggerAxios.js
Outdated
@@ -0,0 +1,28 @@ | |||
import axios from "axios"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, @khalifan-kfan, instead of creating a new file and creating duplicate code, try to do something I did in the same axios file here
} | ||
|
||
return axios | ||
console.log(ACTIVITY_LOGS_API_URL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if this console logger is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice refactor @Mubangizi , this has been added, thank you
Code Climate has analyzed commit 6db10bf and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 41.3% (70% is the threshold). This pull request will bring the total coverage in the repository to 35.4% (0.1% change). View more on Code Climate. |
Description
This PR is to change the base URL for activity logs to the new microservice URL
Type of change
Trello Ticket ID
https://trello.com/c/Gazjtbbq
How Can This Been Tested?
Run this application and add the environment variable
REACT_APP_ACTIVITY_LOGS_API_URL
that should be the base URL to the new app logger activitiesChecklist: