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

DEV-127: replace fetch calls with axios #480

Open
wants to merge 21 commits into
base: development
Choose a base branch
from
Open

DEV-127: replace fetch calls with axios #480

wants to merge 21 commits into from

Conversation

mpark63
Copy link
Collaborator

@mpark63 mpark63 commented Apr 8, 2023

description

for sake of consistency, replace all uses of fetch with axios

Also, for DEV-126 Refactor API calls to userService

implementation

  • use axios
  • handle errors and toast

for DEV-126:

Refactored API calls:

  • GET /thread/getByPlan/:id
    - Refactored the call in RoadmapComment.tsx with getThreads
    from userService
  • GET /plansByUser
    - Wrote getPlanByUser in userService
    - Refactored the call in HandleUserInfoSetUpDummy.tsx with
    getPlanByUser
  • POST /plans
    - Wrote createNewPlan in userService
    - Refactored the call in GenerateNewPlan.tsx
  • GET /years/:id
    - Wrote getYear in userService
    - Refactored the call in HandlePlanShareDummy.tsx
  • DELETE /plans/:id
    - Wrote deletePlan in userService
    - Refactored the call in DeletePlanPopup.tsx
  • POST /thread/reply
    - Refactored the call in Comment.tsx with postNewComment
    function in userService
  • POST /thread/new
    - Refactored the call in NewComment.tsx with postNewThread
    function in userService
  • DELETE /notifications/:id
    - Wrote deleteNotifications function in userService
    - Refactored the call in Notification.tsx
  • PATCH /years/updateYear
    - Wrote updateYear in userService
    - Refactored the call in YearSettingsDropdown.tsx

Not refactored into userService:

  • GET /plans/:id
    - error handling creates a toast
  • GET /courses/:id
    - request has ContentType header in addition to Authorization
    header
  • GET /cartSearch
    - empty array returned in error handling
  • GET /search
    - setSearching(false) called in error handling
  • GET /searchByNumber
    - No longer exists in updated DEV-127 branch
  • POST /planReview/changeStatus
    - Error handling creates a toast
  • PATCH /years/updateName
    - Has ContentType header in addition to authorization header
  • PATCH /courses/dragged
    - Has ContentType header in addition to authorization header
  • GET /user
    - Has ContentType header in addition to authorization header
  • DELETE /verifyLogin
    - Not worth refactoring because attempt to refactor would lead
    to more lines of code than what already exist
  • GET /getYearRange
    - Not worth refactoring because attempt to refactor would lead
    to more lines of code than what already exist

@vercel
Copy link

vercel bot commented Apr 8, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ucredit-distributions-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 26, 2023 2:24am
ucredit-frontend-typescript ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 26, 2023 2:24am
ucredit-frontend-typescript-local ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 26, 2023 2:24am

Copy link
Collaborator Author

@mpark63 mpark63 left a comment

Choose a reason for hiding this comment

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

Share plan fails and droppable thing doesn't work anymore
@josephined21 help?

Comment on lines 85 to 88
axios
.patch(getAPI(window) + '/plans/update', body, {
headers: { ContentType: 'application/json', Authorization: `Bearer ${token}` },
})
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

update plan name / majors

Comment on lines +183 to +186
axios
.patch(getAPI(window) + '/years/changeOrder', body, {
headers: {
'Content-Type': 'application/json',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Screenshot 2023-04-07 at 11 38 16 PM

???????

Copy link
Collaborator

Choose a reason for hiding this comment

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

has this been addressed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

"no" - minseo

@@ -232,32 +231,30 @@ const CourseList: FC<Props> = ({ mode }) => {
newTerm: destination.semester,
};

let res: any = await fetch(getAPI(window) + '/courses/dragged', {
method: 'PATCH',
let res = await axios.patch(getAPI(window) + '/courses/dragged', body, {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

dragging courses to new semester works
error check too

Comment on lines +212 to +216
axios
.post(getAPI(window) + '/courses', body, {
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${token}`,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

adding course from Semester works

Comment on lines +205 to +211
axios
.patch(getAPI(window) + '/years/updateName', body, {
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${token}`,
},
})
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

works

if (planList[i]._id === newPlan._id) {
newPlanList[i] = newPlan;
}
const handlePostAddCourse = (plan: Plan) => (data): void => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

add course to update

}
});
newPlan = { ...currentPlan, years: years };
axios
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

delete course works

Comment on lines 38 to 43
axios
.delete(getAPI(window) + '/plans/' + currentPlan._id, {
headers: {
Authorization: `Bearer ${token}`,
},
})
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

delete plan works

Authorization: `Bearer ${token}`,
},
})
axios
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

delete year works

if (error.status === 400) {
toast.error(error.detail);
}
try {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

add course from coursesearch works

@mpark63 mpark63 requested a review from josephined21 April 8, 2023 03:56
Copy link
Collaborator

@mattliu-mygit mattliu-mygit left a comment

Choose a reason for hiding this comment

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

One comment above ^.^

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

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.

3 participants