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

FinalGroupCapstone-pullRequest #7

Merged
merged 36 commits into from
Feb 15, 2024
Merged

FinalGroupCapstone-pullRequest #7

merged 36 commits into from
Feb 15, 2024

Conversation

IvonneBenitesRodriguez
Copy link
Owner

Requirements:

User logs into website.
Navigation Panel.
Main Page with a list of items.
Item's detail page with description. Reserve button is updated.
Add item link in the navigation panel.
Responsive site for mobile and desktop version
Linters added and passed.

Backend

Copy link

@Meltrust Meltrust left a comment

Choose a reason for hiding this comment

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

Hi,

Good job so far!
There are some issues that you still need to work on to prepare your project for the final evaluation, but you are almost there!

To highlight:

  • Frontend tests are passing✔️
  • Nice code organization ✔️
  • Good readme ✔️
  • Beautiful frontend ✔️

You are really close to finishing the Microverse program!! Keep it up! 👍👍👍

You can do it

After implementing the requested changes, please submit another review request. ♻️

Check the comments under the review.

Cheers and Happy coding!👏👏👏

Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the previous reviews unless it is requested otherwise.


README.md Outdated
Comment on lines 122 to 134

To run tests, run the following command:


```sh
rspec spec --exclude-pattern "spec/integration/api/*_spec.rb"
```
To test the API documentation, run the following command:

```sh
rake rswag:specs:swaggerize
```

Choose a reason for hiding this comment

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

  • You have included frontend tests, and they are passing. Kindly, include the instructions here to run your frontend tests npm run test.. 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okey
Thanks

Comment on lines +118 to +120
npm start
```

Choose a reason for hiding this comment

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

  • This command is not going to work:

image

The correct command according to your package.json file is npm run dev:

image

Kindly, correct these instructions. 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okey
Thanks

@@ -0,0 +1,43 @@
import PropTypes from 'prop-types';

const Reservation = ({

Choose a reason for hiding this comment

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

  • OPTIONAL: When I make a new reservation, it didn't show up right away in my reservations:

image

The only way to view my new reservations is reloading the page and logging in again.

This is because a new GET request needs to be sent to the server to update the list of reservations for the user in the frontend.

This needs to happen either when a new reservation is created, or when visiting 'my reservations'.

Kindly fix this so your users don't have to login again in order to view their new reservations. 👍

note: When adding more new hotels this issue didn't persist. This is therefore optional. Please just take a look.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okey
Thanks

import PlaceModal from '../components/PlaceModal';
import '../styles/deletePlace.css';

const DeletePlace = () => {

Choose a reason for hiding this comment

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

  • When I delete a hotel that has been reserved, and then immediately go to "my reservations" the app crashes:

image

image

The only way to recover it is by reloading the page and logging in again.

Kindly, take a look at this issue. 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okey
Thanks

Copy link

@Meltrust Meltrust left a comment

Choose a reason for hiding this comment

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

Hi,

Good job so far!
There are some issues that you still need to work on to prepare your project for the final evaluation, but you are almost there!

To highlight:

  • Readme has been improved✔️

You are really close to finishing the Microverse program!! Keep it up! 👍👍👍

You can do it

After implementing the requested changes, please submit another review request. ♻️

Check the comments under the review.

Cheers and Happy coding!👏👏👏

Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the previous reviews unless it is requested otherwise.


import ReservationsContainer from '../components/ReservationsContainer';
import ReservationsSlider from '../components/ReservationsSlider';

const MyReservations = () => {

Choose a reason for hiding this comment

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

  • When I delete a hotel that has been reserved, and then immediately go to "my reservations" the app still crashes:

image

This is because after deleting a hotel, a new request to the server needs to be done in order to fetch the updated list of reservations in case that the deleted hotel was reserved.👍

Copy link
Owner Author

Choose a reason for hiding this comment

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

Dear @Meltrust ,
Very nice to meet you,
we are fixing the issue you mentioned,
for us it is working, can you please give us an quick orientation 🙏🏿,
thank you,
Best wishes,

Choose a reason for hiding this comment

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

Of course, the app crashes when a hotel that has been reserved is deleted, and then you go to "my reservations."

This is because when you delete a hotel that has been reserved, that reservation is also deleted:

image

Since the hotel that was reserved has been deleted if you right away visit the "my reservations" section it crashes:

image

The solution is to fetch from the server the new list of hotels and reservations any time the number of hotels or reservations changes.

Therefore, after deleting a hotel, you can fetch the updated list of reservations, and the problem will be solved. This needs to happen after a hotel has been deleted by pressing the button.

First, make the server delete the hotel and THEN fetch the updated list of hotels/reservations.

That way, if the user visits "my reservations" they will be updated already and the app won't crash.

@gilberthappi gilberthappi merged commit a029f3e into main Feb 15, 2024
3 checks passed
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.

None yet

3 participants