Update video states #159

IrynaLypnyk · 2023-11-29T23:30:56Z

Ticket https://github.com/JoeWeate/thebigmouth/issues/139

added apiUpdateVideo axios request function
created constants for all video actions, states
added function to validate video state update
created VideoActionButton component with all buttons view and logic
add Snackbar component and added for video state updates

To use VideoActionButton you need to add <VideoActionButton key={key} videoData={someVideoData} action={actionName}/> - options for possible actionNames are in constant ACTION_NAME in src/utils/constants.js

All types of buttons temporary placed at the bottom of our DummyHomePage. Pay attention that you will see all the buttons - for admin and for user as userRole is fake(userRole === USER_ROLE.USER, userRole !== USER_ROLE.ADMIN) but their working logic as for real user

…ponent

…st for testing

sherif98 · 2023-11-30T19:36:58Z

src/pages/DummyHomePage/index.js

Can you please merge the latest main? I think we treat this component as the actual home page now

You are right. Tank you. Removed all buttons examples from that page.
Added to comment above "To use VideoActionButton you need to add <VideoActionButton key={key} videoData={someVideoData} action={actionName}/> - options for possible actionNames are in constant ACTION_NAME in src/utils/constants.js"

sherif98 · 2023-11-30T19:40:28Z

src/utils/constants.js

+};
+
+export const VIDEO_STATE = {
+    DRAFT: 'draft', //[USER: prevState === null (just uploaded)],


The comment here is not entirely correct, right? It could be that the video was moved to draft

sherif98 · 2023-11-30T19:40:51Z

src/utils/constants.js

+    {//DELETE
+        action: ACTION_NAME.DELETE,
+        currentState: VIDEO_STATE.DRAFT,
+        nextSate: null,//do we keep it in DB?


I think delete is a delete, we should remove the record from the database. Let's leave this for now and implement the delete functionality later.

Yes, I remember that we discussed this and agreed to not to keep them in DB, I just forgot to delete the comment.

sherif98 · 2023-11-30T19:42:30Z

src/utils/constants.js

+        nextSate: VIDEO_STATE.DRAFT
+    },
+    {//PUT
+        action: ACTION_NAME.MOVE_TO_PENDING,


nit: Should we rename this action to SEND_FOR_REVIEW? What do you think? We might then rename the pending state to IN_REVIEW as well.

Super!. SEND_FOR_REVIEW is much better.

sherif98 · 2023-11-30T20:00:47Z

src/utils/validateChangeState.js

@@ -0,0 +1,40 @@
+import { VIDEO_DATA_KEYS, USER_ROLE, VIDEO_STATE, ACTION_NAME } from './constants';
+
+export const validateChangeState = ({videoData, action, userRole}) => {


I think we can simplify this function by reusing the logic you've implemented in constants.js.

Imagine that we have an object that looks like the following:

videoStates = { "user": { "MOVE_TO_PENDING": { "validate": (currentState) => currentState == DRAFT, "nextState": "PENDING" }, "MOVE_TO_DRAFT": {....}, // other actions }, "admin": { "APPROVE": { "validate": (currentState) => currentState == PENDING "nextState": "APPROVED" }, // other actions }

Then we can use this object in the validateChangeState function as the following:

val state = videoStates[user.role][action] val isValid = state["validate"](videoData[VIDE_DATA_KEYS.STATE]) if (isValid) state["nextState"]

I think this will allow us to have the logic defined in the videoStates object and the validateChangeState function will be simple without lots of nested if conditions. Let me know what you think.

@sherif98 it sounds great. It really should be much simpler and more readable. I'll do it now. Thank you so much for your review 🫶

sherif98 · 2023-11-30T20:02:11Z

src/utils/validateChangeState.js

+            if (userRole === USER_ROLE.USER && currentVideoState === VIDEO_STATE.PENDING) {
+                nextState = VIDEO_STATE.APPROVED;


Is this correct? Can a user approve a video?

Of course it is not correct 😱. The right code should be

if (action === ACTION_NAME.APPROVE) { if (userRole === USER_ROLE.ADMIN && (currentVideoState === VIDEO_STATE.IN_REVIEW)) { nextState = VIDEO_STATE.APPROVED; }

Thank you!

IrynaLypnyk · 2023-12-01T12:33:32Z

@sherif98 thank you very much for your review 🫶 I've updated my code according to your comments. Hope very much you will have time to look through it once again with my updates.

sherif98

LGTM! Looks great now Iryna!

sherif98 · 2023-12-02T11:47:56Z

src/api/videos.js

@@ -31,3 +31,18 @@ export const getAllVideoByUserID = (UserID) => {
      console.log(error);
    });
 };
+
+export const apiUpdateVideo = (updatedVideo, handleSnackbar) => {


nit: rename handleSnackbar to a more generic name, for example handleStatus or reportStatus. The reason is that in API level (in videos.js) it doesn't need to know that you're using a snackbar or an error message or whatever to display something to user, it should be agnostic on the API level

sherif98 · 2023-12-02T11:49:19Z

src/components/userHub/SelectSortBy.js

Please merge the latest main and update the PR. I don't think this file is newly added here, it was added in the SortBy PR

sherif98 · 2023-12-02T11:52:29Z

src/utils/validateChangeState.js

+        if(isValid) {
+            return VIDEO_STATE_UPDATE_ACTIONS[userRole][action].nextSate;
+        } else {
+            return false;


In the if branch we return a string (the state) while here we return a boolean, can we unify the return type of the function? Maybe make it return undefined here or so?

What do you think?

Bedi06 · 2023-12-02T12:25:02Z

src/components/UpdateVideoStateButton.js

well-written and structured code makes it easily readable, good use of the use context from react for accessing of the user role

Bedi06 · 2023-12-02T12:35:21Z

src/components/UpdateVideoStateButton.js

+                    //navigate to somewhere
+                });
+            } else if(action === ACTION_NAME.DELETE){
+                console.log('add delete confirmation and axios request')


It seems like you've handled everything well. Great job! Perhaps, in the future, we could consider incorporating a pop-up message or a text that says, "Are you sure you want to delete this video?" This will provide a double confirmation with a Yes or No option

AndriusIsin

👏 Great job! You've done an excellent job with this pull request! 👏

IrynaLypnyk added 5 commits November 30, 2023 01:03

[video update]: add api, mocks and constants

22dc5ec

[video update]: add Snackbar component

b8f9873

[video update]: add validateChangeState function

cc9a302

[video update]: add Button constants and create VideoActionButton com…

3be3f2d

…ponent

[video update]: add all action buttons to DummyHomePage temporally ju…

8ff17fe

…st for testing

IrynaLypnyk added the front end label Nov 29, 2023

IrynaLypnyk requested review from JoeWeate, sherif98, AndriusIsin, OlhaDanylevska and Bedi06 November 29, 2023 23:30

IrynaLypnyk self-assigned this Nov 29, 2023

IrynaLypnyk changed the base branch from main to li-add-context-provider November 29, 2023 23:33

sherif98 reviewed Nov 30, 2023

View reviewed changes

IrynaLypnyk added 3 commits December 1, 2023 12:32

Merge main branch into li-videos-update-functions branch

fdea385

[video states]: rename constant and action, fix logic bug

11aa653

[video states]: updated logic to validate video state updates

b7b6037

[video states]: fix comments in constants.js

48705ad

sherif98 approved these changes Dec 2, 2023

View reviewed changes

Bedi06 approved these changes Dec 2, 2023

View reviewed changes

AndriusIsin approved these changes Dec 2, 2023

View reviewed changes

IrynaLypnyk merged commit e5bcfe4 into li-add-context-provider Dec 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update video states #159

Update video states #159

IrynaLypnyk commented Nov 29, 2023 •

edited

Loading

sherif98 Nov 30, 2023

IrynaLypnyk Dec 1, 2023 •

edited

Loading

sherif98 Nov 30, 2023

sherif98 Nov 30, 2023

IrynaLypnyk Dec 1, 2023

sherif98 Nov 30, 2023

IrynaLypnyk Dec 1, 2023

sherif98 Nov 30, 2023

IrynaLypnyk Dec 1, 2023

sherif98 Nov 30, 2023

IrynaLypnyk Dec 1, 2023

IrynaLypnyk commented Dec 1, 2023

sherif98 left a comment

sherif98 Dec 2, 2023

sherif98 Dec 2, 2023

sherif98 Dec 2, 2023

Bedi06 Dec 2, 2023

Bedi06 Dec 2, 2023 •

edited

Loading

AndriusIsin left a comment

		@@ -0,0 +1,40 @@
		import { VIDEO_DATA_KEYS, USER_ROLE, VIDEO_STATE, ACTION_NAME } from './constants';

		export const validateChangeState = ({videoData, action, userRole}) => {

		if (userRole === USER_ROLE.USER && currentVideoState === VIDEO_STATE.PENDING) {
		nextState = VIDEO_STATE.APPROVED;

Update video states #159

Update video states #159

Conversation

IrynaLypnyk commented Nov 29, 2023 • edited Loading

Choose a reason for hiding this comment

IrynaLypnyk Dec 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IrynaLypnyk commented Dec 1, 2023

sherif98 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bedi06 Dec 2, 2023 • edited Loading

Choose a reason for hiding this comment

AndriusIsin left a comment

Choose a reason for hiding this comment

IrynaLypnyk commented Nov 29, 2023 •

edited

Loading

IrynaLypnyk Dec 1, 2023 •

edited

Loading

Bedi06 Dec 2, 2023 •

edited

Loading