-
Notifications
You must be signed in to change notification settings - Fork 0
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
Space Travelers Project #35
Conversation
Space-Travelers: Initialize project
Rockets page structure setup review
Lists render Review
…tion book/cancel Reservation Review
Display Joined Missions in My Profile page.
Adding unit tests using React Testing Library.
Display Reserved Rockets Review
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.
Hi 🙋 🙋 @kessie2862, @akezeth
Good job so far!
There are some issues that you still need to work on to go to the next project but you are almost there!
Highlights 🟢
- Great job enhancing the My Profile section by adding a placeholder text when no rocket or mission is joined 🚀
Required Changes ♻️
N/A
Check the comments under the review.
Optional suggestions
N/A
Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.
Cheers and Happy coding!👏👏👏
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.
As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.
README.md
Outdated
# 📖 Bookstore <a name="about-project"></a> | ||
|
||
**Bookstore** Working with the real live data from the SpaceX API by building a web application for a company that provides commercial and scientific space travel services. The application will allow users to book rockets and join selected space missions. |
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.
- Let's kindly edit the title of our project. This is not a bookstore project.
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've fixed that.
src/redux/rocket/rocketSlice.js
Outdated
name: 'Rockets', | ||
initialState: { | ||
rockets: [], | ||
// bookedRockets: [], |
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.
- Comments render our codebase messy. Let's kind get rid of all commented codes.
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've removed all unwanted comments. Thanks
src/redux/rocket/rocketSlice.js
Outdated
// const rocket = state.rockets.find( | ||
// (rocket) => rocket.id === rocketId, | ||
// ); | ||
// if (rocket) { | ||
// rocket.reserved = true; | ||
// state.bookedRockets.push(rocket); | ||
// } |
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.
- Please do not push commented code to the repo. It's ugly.
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.
Well noted.
src/components/Missions.js
Outdated
const fetchMissions = async () => { | ||
try { | ||
const response = await fetch('https://api.spacexdata.com/v3/missions'); | ||
const data = await response.json(); | ||
dispatch(setMissions(data)); | ||
} catch (error) { | ||
console.error('Error fetching missions:', error); | ||
} | ||
}; | ||
|
||
fetchMissions(); |
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 think components should simply wait around to handle data from redux where the app state is being managed. Hence this logic should have been done as in Rockets in rocketSlice. Components should only call the API module directly and then dispatch the action.
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.
Implemented those changes
<div className="profile-container"> | ||
<div className="missions-profile"> | ||
<h2>My Missions</h2> | ||
<div className="joined-missions"> | ||
{joinedMissions.length === 0 ? ( | ||
<p>No missions joined.</p> | ||
) : ( | ||
<ul> | ||
{joinedMissions.map((mission) => ( | ||
<li key={mission.mission_id}>{mission.mission_name}</li> | ||
))} | ||
</ul> | ||
)} | ||
</div> | ||
</div> | ||
<div className="rockets-profile"> | ||
<h2>My Rockets</h2> | ||
<div className="booked-rockets"> | ||
{bookedRockets.length === 0 ? ( | ||
<p>No rockets booked.</p> | ||
) : ( | ||
<ul> | ||
{bookedRockets.map((rocket) => ( | ||
<li key={rocket.id}>{rocket.rocket_name}</li> | ||
))} | ||
</ul> | ||
)} | ||
</div> | ||
</div> | ||
</div> | ||
); | ||
} |
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.
- [Optional] I think this section can equally be enhanced by adding the "Cancel reservation" and "Leave Mission" buttons close to each reserved rock or mission joined. Clicking them should dispatch the same actions for both the rocket and the mission. This way, a user can cancel a reservation even on this page without going back to the rocket page.
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 think we will look into that later. Thanks.
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.
Hi team,
Your project is complete! There is nothing else to say other than... it's time to merge it 🥳
Congratulations! 🎉
Status: Approved✔️
Highlights
- No linter errors
- Descriptive PR
- Good follow-up with the previous reviewer.
Optional suggestions
Every comment with the [OPTIONAL] prefix won't stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better. Some of them were simply missed by the previous reviewer and addressing them will really improve your application.
Cheers and Happy coding!👏👏👏
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.
As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.
import React, { useEffect } from 'react'; | ||
import { useDispatch } from 'react-redux'; | ||
import { useSelector } from 'react-redux/es/hooks/useSelector'; | ||
import { fetchRockets } from '../redux/rocket/rocketSlice'; | ||
import Rocket from './Rocket'; | ||
|
||
function Rockets() { | ||
const rocketItems = useSelector((state) => state.rocket.rockets); | ||
const dispatch = useDispatch(); | ||
useEffect(() => { | ||
dispatch(fetchRockets()); | ||
}, [dispatch]); | ||
return ( | ||
<div className="container"> | ||
{rocketItems.map((rocket) => ( | ||
<Rocket | ||
key={rocket.id} | ||
id={rocket.id} | ||
image={rocket.image} | ||
name={rocket.rocket_name} | ||
description={rocket.description} | ||
reserved={rocket.reserved} | ||
/> | ||
))} | ||
</div> | ||
); | ||
} | ||
|
||
export default Rockets; |
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.
- Optional
The rocket reservation data does not persist when I navigate to the profile and back. Since the previous reviewer didn't raise this as required, I am marking this as optional. However, I would recommend you implement this. It would be a good challenge for debugging and learning more about redux.
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 recommendation. We will work on that to persist the rocket reservation data. Thanks
Description.
This PR includes the implementation of
rockets
andmissions
features for the application. The changes enable fetching and displaying data from the SpaceX API, implementing actions for rocket booking and mission joining, as well as canceling reservations and leaving missions. Additionally, aMy Profile
page is added to render a list of joined missions and reserved rockets.Changes made.
Created app and installed basic dependencies using
Create React App
and installedReact Redux
andReact Router
.Created website's header with an empty
navigation
section.Implemented the basic structure for rockets and missions, including a route and a view component. Utilized
<NavLink />
for page navigation links and styled the active class to visually indicate the current section/page.Fetched
missions
androcket
data from theSpaceX API
.Displayed rockets and missions using lists. Utilized the
useSelector()
Redux Hook to select relevant state slices and render the lists of rockets and missions in their respective routes.Implemented actions for
rocket booking
andmission joining
by dispatching an action to update the store when the user clicks the "Reserve rocket
" or "Join Mission
" buttons.Switched badges for rockets and missions. Reserved rockets now display a "Reserved" badge and a "Cancel reservation" button, while joined missions show an "Active Member" badge and a "Leave Mission" button.
Implemented rocket booking cancellation and mission leaving. Dispatched corresponding actions upon clicking the respective buttons.
Rendered a list of all joined missions and reserved rockets on the
My Profile
page.Added unit tests with React Testing Library and
jest
.Included styles to make the
UI
visually attractive