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

London 10 | Maksim Lukianenko | Fullstack Project Assignment | Level 999 #337

Closed
wants to merge 12 commits into from

Conversation

Mpanasetckiy
Copy link

@Mpanasetckiy Mpanasetckiy commented Aug 18, 2023

I completed the project using JS, React, Node, and MongoDB via Mongoose. While building the project, I utilised components from previous projects, such as the Modal window and custom http-hook. Also for better consistency and security reasons, I created .env files to store API key and other data in both front and back ends without pushing them online.

feat(App.css): add center alignment to p element in container class
feat(Modal.jsx): create Modal component for deleting bookings
feat(Clip.jsx): create Clip component for displaying videos
feat(Main.jsx): replace static video card with Clip component
feat(http-hook.js): create custom hook for handling HTTP requests
feat(package.json): add start script for nodemon
feat(routes.js): create routes for getting videos
feat(server.js): add CORS, MongoDB connection, and routes to server
feat(routes.js): update GET / route to retrieve all videos from MongoDB collection
refactor(Clip.jsx): remove unused code and replace iframe with ReactPlayer component
feat(Container.jsx): create Container component to fetch and display videos
refactor(Main.jsx): replace Clip component with Container component
feat(App.css): add styles for .video__header and .close-button
feat(Modal.css): add styles for .modal and .modal__container
fix(Modal.jsx): change text from "Delete booking?" to "Delete video?"
feat(Clip.jsx): add functionality to delete video and change rating
fix(Container.jsx): remove unused state variable
feat(routes.js): add DELETE route to delete video by id
fix(Clip.jsx): remove useEffect import from react
fix(Clip.jsx): remove unused console.log statements
fix(Clip.jsx): change class attribute to className in JSX
fix(Clip.jsx): add missing semicolons to function calls
fix(Clip.jsx): update currentClip state when rating changes
fix(Clip.jsx): update rating in updateRating function
fix(Container.jsx): remove unused import statement
feat(Container.jsx): add removeVideoLocally and changeRating functions
feat(Container.jsx): update videos state when removing or changing rating
fix(routes.js): remove unused console.log statement
feat(Clip.jsx): add support for updating rating locally and remotely
fix(Container.jsx): fix useState initialization for currentClip
feat(Container.jsx): add support for updating rating locally and remotely
feat(routes.js): add PATCH route for updating video rating
…ntainer component

fix(client): fix typo in Search component filename
fix(client): fix typo in Search component placeholder
fix(client): fix typo in Search component handleChange function
fix(client): fix typo in Search component handleAdding function
fix(client): fix typo in Search component addVideo function
fix(client): fix typo in Container component filename
fix(client): fix typo in Container component setCurrentClip function
fix(client): fix typo in Container component removeVideoLocally function
fix(client): fix typo in Container component changeRating function
fix(client): fix typo in Container component changeRatingLocally function
fix(client): fix typo in Clip component filename
fix(client): fix typo in Clip component handleDelete function
fix(client): fix typo in Clip component handleHover function
fix(client): fix typo in Clip component changeRating function
fix(client): fix typo in Clip component return statement
fix(client): fix typo in Clip

fix(routes.js): fix formatting and add missing semicolons
feat(routes.js): add POST route to create new video clip
…cts with other services

fix(server.js): update database connection URL to use the correct environment variables
fix(routes.js): update the findOneAndUpdate method to return the updated document by setting the 'new' option to true
feat(UploadComponent.jsx): add functionality to add a new video by sending a POST request to the server and updating the local state
feat(Search.jsx): create a search component to filter videos based on a search query
refactor(App.js): remove Selection component as it is no longer needed
refactor(Container.jsx): pass the setData function as a prop to update the data state in App.js when videos are fetched from the server
…o improve UI

fix(App.js): change variable name from data to videos for clarity and consistency
feat(Container.jsx): add sortOption state to manage sorting option and handleSortOptionChange function to sort videos based on selected option
…formance and loading speed

fix(App.css): update color variables and font family to improve consistency and readability
feat(App.js): add Footer component to the main App component for better user experience and branding
fix(Modal.css): update color variables and font family to improve consistency and readability
fix(Modal.jsx): update button text and styling for better user experience
fix(Container.jsx): remove unused sortOption state variable
feat(Footer.jsx): create Footer component with CYF logo and attribution text
fix(UploadComponent.jsx): add validation for required fields and handle error when adding video
Copy link
Member

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

This generally looks really nicely put together, good job!

import Footer from "./components/Footer";

const App = () => {
const [data, setData] = useState([]);
Copy link
Member

Choose a reason for hiding this comment

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

"data" is a very generic name for a piece of state - can you think of a name that better describes what data is stored here?


const searchForVideo = () => {
const filteredVideos = data.filter((video) => {
return video.title.toLowerCase().includes(searchQuery);
Copy link
Member

Choose a reason for hiding this comment

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

Is the search query guaranteed to be lower case? If not, what will happen if I search for "Hello"?

const filteredVideos = data.filter((video) => {
return video.title.toLowerCase().includes(searchQuery);
});
setVideos(filteredVideos);
Copy link
Member

Choose a reason for hiding this comment

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

I would generally expect you not to discard the original list of videos, because ideally you wouldn't need to re-fetch them if I remove/change my search query... Can you think how to avoid throwing away the full list of videos when searching?

Comment on lines +35 to +36
setCurrentClip({ ...currentClip, rating: currentClip.rating + 1 });
updateRating(currentClip.id, currentClip.rating + 1);
Copy link
Member

Choose a reason for hiding this comment

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

It's interesting to need both of these calls - I would kind of expect just one function to be passed as a prop to update rating, and that would update all of the things needed - what do you think?

const UploadComponent = ({ addVideo }) => {
const randomId = uuidv4();
const [videoData, setVideoData] = useState({
id: randomId,
Copy link
Member

Choose a reason for hiding this comment

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

I'd generally expect the server to be generating IDs (because it can guarantee they're actually unique), rather than trusting the client to give it a unique value.

});
console.log(response);
} catch (err) {
console.log("Error adding video.", { error: err });
Copy link
Member

Choose a reason for hiding this comment

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

A user will never see this error - how can you ensure the user actually knows when something went wrong, and what they should do about it?

@Dedekind561 Dedekind561 closed this Apr 8, 2024
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