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

Space Travelers Project #35

Merged
merged 72 commits into from
Jun 29, 2023
Merged

Space Travelers Project #35

merged 72 commits into from
Jun 29, 2023

Conversation

kessie2862
Copy link
Owner

Description.

This PR includes the implementation of rockets and missions 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, a My 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 installed React Redux and React 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 and rocket data from the SpaceX 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 and mission 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

Copy link

@FrederickMih FrederickMih left a 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
Comment on lines 29 to 31
# 📖 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.

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.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I've fixed that.

name: 'Rockets',
initialState: {
rockets: [],
// bookedRockets: [],

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.

Copy link
Owner Author

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

Comment on lines 27 to 33
// const rocket = state.rockets.find(
// (rocket) => rocket.id === rocketId,
// );
// if (rocket) {
// rocket.reserved = true;
// state.bookedRockets.push(rocket);
// }

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.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Well noted.

Comment on lines 16 to 26
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();

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.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Implemented those changes

Comment on lines +13 to +44
<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>
);
}

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.

Copy link
Owner Author

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.

Copy link

@muneebulrehman muneebulrehman left a 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.

Comment on lines +1 to +29
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;

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.

Copy link
Owner Author

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

@akezeth akezeth merged commit 691de09 into main Jun 29, 2023
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.

4 participants