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

All TODO's complete #131

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

roushan-code
Copy link

@roushan-code roushan-code commented Jun 24, 2024

Hi sir, I have completed all the todos and learned many things.

I have also tested all todos with Postman.

Thanks for giving me such a great learning opportunity.

@Slashz07
Copy link

can you please tell me why is userId required in getAllVideos in video controller and how can it be accessed and sent in req.query.
I just cant understand, since req.query accesses key-value pairs from url ,which are set by the submission of form where user would put his search query...so from where does the userId come from

@roushan-code
Copy link
Author

roushan-code commented Jul 14, 2024

User-Id is required because with this route the user will get all the videos that he uploads. so the user-ID is required for authentication.

There is a condition that if the user is logged in then He will get all his uploaded videos based on query. That means the User-Id will send from frontend to backend. So that User verification should be done through backend.

frontend code will be like this ->

import axios from 'axios';

const getAllVideos = async (id) => {
try {
const response = await axios.get('/api/videos', {
params: {
page: 1,
limit: 10,
query: '',
sortBy: 'createdAt',
sortType: 1,
userId: id
}
});
const { data } = response;
console.log(data);
// render video list
} catch (error) {
console.error(error);
}
};

---------OR------------

export const getAllVideos = (query= "", page= 1, sortBy= 'createdAt', sortType=1, userId= id) => async (dispatch) => {
try {
dispatch({ type: ALL_VIDEO_REQUEST });
let link = /api/v1/videos?keyword=${query}&page=${page}&sortBy=${sortBy}&sortType=${sortType}&userId=${userId};

    const { data } = await axios.get(link);
    dispatch({
        type: ALL_VIDEO_SUCCESS,
        payload: data,
    })
} catch (error) {
    const errorMessage = error.response ? error.response.data.message : "An error occurred";
    dispatch({
        type: ALL_VIDEO_FAIL,
        payload: errorMessage,
    });
}

};

@Slashz07
Copy link

Slashz07 commented Jul 14, 2024

i think the feature of a user getting all of his own uploaded videos should be in another feature since there is no need of any query in that but sir has commented to use userQuery...so shouldn't a separate controller be made for that since any $match or $search would not be needed due to absence of any user query

also pls tell from if the id in
const getAllVideos = async (id) => {
try {
....
} is being accessed from the user data that was returned when user logged in at first...

btw..thnx a lot for replying..i dont have any person to solve my doubts in person..its just self learning..i appreciate your response

@roushan-code
Copy link
Author

roushan-code commented Jul 15, 2024

I think the feature of a user getting all of his own uploaded videos should be in another feature since there is no need of any query in that but sir has commented to use userQuery...so shouldn't a separate controller be made for that since any $match or $search would not be needed due to absence of any user query

I have checked the code. I think you are right. User_id should be optional for getting video. Because the user who doesn't log in is also able to get videos according to query. but if the user is logged in then the user-id could pass from frontend to check what types of videos the user prefers. so that if any algorithm will be applied in the future to check what kinds of videos users prefer then they can serve videos according to their preference (like on youtube, facebook etc.).

I have applied user required at that time because of this line in video.routes.js file --> router.use(verifyJWT); // Apply verifyJWT middleware to all routes in this file
The above router line is written by Sir .

also pls tell from if the id in
const getAllVideos = async (id) => {
try {
....
} is being accessed from the user data that was returned when user logged in at first...

yes

Now I have updated the code. Thanks for your suggestion.

@Slashz07
Copy link

I have applied user required at that time because of this line in video.routes.js file --> router.use(verifyJWT); // Apply verifyJWT middleware to all routes in this file The above router line is written by Sir .

but if we apply verifyjwt to alll routes, any user who isnt logged in wont be able to search any videos since verifyjwt would reject access to that user as there wont be any tokens in cookies for a not-logged-in user...
is it that sir is focusing on only allowing access to logged in users??

@roushan-code
Copy link
Author

roushan-code commented Jul 15, 2024

but if we apply verifyjwt to alll routes, any user who isnt logged in wont be able to search any videos since verifyjwt would reject access to that user as there wont be any tokens in cookies for a not-logged-in user... is it that sir is focusing on only allowing access to logged in users??

may be sir think so.

But I have also updated the video routes in my code

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