hey @Bennylin thanks for raising the query. As the tool is undergoing some changes and development, we will try to fasttrack this query asap

here's a patch which has similar workings, and shall clear this out
https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/+/890137

Thanks for raising the query

cc @Gopavasanth @Soda @Punith.nyk

Hi @dipanjan_sengupta I would strongly advise you to pick one ticket at a time. Lets not hoard on the tickets. Lets give everyone a chance to pick up the tasks.
Removing you from assignee here

In T338465#10554660, @Reputation22 wrote:

Hi @dipanjan_sengupta I would strongly advise you to pick one ticket at a time. Lets not hoard on the tickets. Lets give everyone a chance to pick up the tasks.
Removing you from assignee here

i picked 1 bug and 1 feature , i can handle it @Reputation22
mistakenly had changes the status of the task , have now fixed the status . if needed u can still assign this to me

In T338465#10554690, @dipanjan_sengupta wrote:

In T338465#10554660, @Reputation22 wrote:

Hi @dipanjan_sengupta I would strongly advise you to pick one ticket at a time. Lets not hoard on the tickets. Lets give everyone a chance to pick up the tasks.
Removing you from assignee here

i picked 1 bug and 1 feature , i can handle it @Reputation22
mistakenly had changes the status of the task , have now fixed the status . if needed u can still assign this to me

lets focus on one thing at a time

I think this can also be marked as good-first-issue.
The resolution here is to change the name of the video when clicked on the Overwrite button.
Also remove the above mentioned fields from the wikitext on clicking Overwrite.

Thank you for tagging this task with good first task for Wikimedia newcomers!

Newcomers often may not be aware of things that may seem obvious to seasoned contributors, so please take a moment to reflect on how this task might look to somebody who has never contributed to Wikimedia projects.

A good first task is a self-contained, non-controversial task with a clear approach. It should be well-described with pointers to help a completely new contributor, for example it should clearly pointed to the codebase URL and provide clear steps to help a contributor get setup for success. We've included some guidelines at https://phabricator.wikimedia.org/tag/good_first_task/ !

Thank you for helping us drive new contributions to our projects <3

@Reputation22 no one else is looking into this , so i tried reproducing it , but unable to do it, need some inputs.

where is the over write button ?
cant find the button .

As you see in the above two pictures, we're on the final stage here (i.e uploading to commons stage), now when we click on Overwrite option, the above input field called Video Title goes away (ensuring user can't change the name, and overwrite can happen) [this is desired]
but within the code videoTitle sets around {video_name}_edited_{index_no}. This causes a new video to get uploaded on commons (as it matches videos by name)
We have to ensure when we click on Overwrite option, it should maintain the same name as video name (and not append the _edited) part at the end.
So that the same video gets overridden.

I hope this answers your query @dipanjan_sengupta

Hello, I've implemented and tested a fix, and have come to the point where I'd like to submit a patch for code review.
I found this documentation for submitting patches on GitLab. As I dont have a developer-level account I looked to fork the repo, but cant seem to find the fork option in the GUI. I've attempted to login to GitLab as well and am "pending approval from your GitLab administrator".

Thanks in advance for any pointers.

@Nicolasmichel: How does the full GitLab page look for you, and what is its full URL? There should be a banner at the top:

Oops I must have dismissed that window in a previous session. I've submitted a unlock request form since I'm not yet in either of the groups. Thank you!

Hello I've implemented a fix to the overwrite / new file bug. The merge request can be found here. Please provide any feedback that you all may have. Thanks!

hey @Nicolosmichel thanks for the MR. I'll review it and get back to you

Thanks

hey @Nicolasmichel have reviewed your MR, please address the comment
Thanks

Hello, thanks for the review! The MR has a subsequent commit with the requested changes + some basic unit tests.

Hey @Nicolasmichel have reviewed your changes..lgtm
hope you've tested the flow thoroughly (both new file upload vs overwrite upload to commons)

hey @Nicolasmichel
as mentioned in ticket description as well, we need to remove this from wikiText in case of overwrite

When I click "Overwrite", first, it shouldn't offer me this templates

and
{{Extracted from|File:blablabla}}

and then, it should overwrite the file, not create a new file
You can refer here: https://commons.wikimedia.org/wiki/File:Unknown_flower_in_Tawangmangu_edited_0.webm
in case of overwrite our source shouldn't be Derived from . Instead it should be source from original video itself
Same for the Extracted from message below

Here are a couple of approaches for this (sorted by pref)

1. remove the field text altogether from the api payload (/upload), in case of overwrite. This is something we should try first, if the api takes in the default (already set up) wikiText in this case, then we don't need to alter anything, and our work here is done. (We would not pass text field in payload for overwrite case)
2. In case the above approach doesn't work, we will remove the above mentioned 2 keys, and would import the other keys from the VideoDetailsContext > videoDetails object, and would keep insert them in the wikiText instead. (This might require refactoring of wikiText code as well, i.e creating a function named generateWikiText() -> handles all wikiText related cases and gives out a final wikiText to be uploaded on commons).

Feel free to add on this / suggest something new which you think might help in this case

Hey,
Yes I tested both new file upload & overwrite. The implementation behaves the same as live.

Ah I must have missed that. Ill give it a crack tomorrow, I need to do some other work rn.

On a side note, I noticed a behavior that struck me as odd while working on this ticket.
If the user chooses to edit the files title and deletes the filename extension (.webm / .ogv) the upload fails.
As long as we know the type of the file we could we not just check to see if the wantTitle has the same extension and insert it back in if the user has deleted it, maybe in the uploadVideo function?

Also, is the intended behavior that when overwriting a file that has already been edited, the index is incremented?
That is:
original upload: video.webm
first edit: video_edited_0.webm
overwrite of first edit: video_edited_1.webm

Because as of now the current implementation simply overwrites with the same name.
original upload: video.webm
first edit: video_edited_0.webm
overwrite of first edit: video_edited_0.webm

In T338465#11360956, @Nicolasmichel wrote:

Hey,
Yes I tested both new file upload & overwrite. The implementation behaves the same as live.

Ah I must have missed that. Ill give it a crack tomorrow, I need to do some other work rn.

On a side note, I noticed a behavior that struck me as odd while working on this ticket.
If the user chooses to edit the files title and deletes the filename extension (.webm / .ogv) the upload fails.
As long as we know the type of the file we could we not just check to see if the wantTitle has the same extension and insert it back in if the user has deleted it, maybe in the uploadVideo function?

i would suggest lets not show extension at all on the videoTitle input bar (so that user never alters it in the first place)
also maybe we can show the extension to the end of the input file (maybe in greyed disabled section) so that it communicates that this extension would be applied on the video title and one can't change it.

we can have this as a seperate ticket, lets not stretch this one. wdys?

In T338465#11361002, @Nicolasmichel wrote:

Also, is the intended behavior that when overwriting a file that has already been edited, the index is incremented?
That is:
original upload: video.webm
first edit: video_edited_0.webm
overwrite of first edit: video_edited_1.webm

Because as of now the current implementation simply overwrites with the same name.
original upload: video.webm
first edit: video_edited_0.webm
overwrite of first edit: video_edited_0.webm

no..so the concept of index is something different.. we have a option within trim manipulation, where you can trim a video and export it as a collection of multiple videos
so the index represents that video index.

also no..thats not the intended behvaiour of overwrite
overwrite essentially means overwriting existing video on commons.. so ideally

1. this shouldn't be avl for videos being uploaded via vct ( since those are not present in commons)
2. when user uploads video from commons (via url), and clicks on overwrite option, we should essentially disable everything on ui (so that user can't manipulate anything) and reset everything with data fetched from commons for that video..so that all metadata is same as commons video and we've overwritten that video with newer version (edited by vct)

we can focus on 2. as of now, and keep 1. as a good to have sort of thing
you can either take it as a part of this ticket, or maybe we can have a new ticket (good first issue) [your call]

Okay, I finally got around to getting some work done on this. (code isnt pushed yet, just looking for some clarification)
I've made some changes such that if the user uploads their own file, they are not presented with overwrite as an option, just upload as new file.
When a user edits an existing file on commons they have both options.

On the wikitext side of things I ended up going with option two and starting to write a generateWikiText function to encapsulate all of the wikitext functionality.
For clarification we have three cases:

1. fresh file -- what do we replace the "derived from" in the source field / what do we populate the source with? Maybe we could make a created by template for VCT?
2. overwriting an existing file -- edit previous wikitext's date and author. Source field remains the same.
3. saving existing file as new -- we are happy with the live version. source field gets "derived from" which points to the parent file.

In T338465#11375616, @Nicolasmichel wrote:

Okay, I finally got around to getting some work done on this. (code isnt pushed yet, just looking for some clarification)
I've made some changes such that if the user uploads their own file, they are not presented with overwrite as an option, just upload as new file.
When a user edits an existing file on commons they have both options.

On the wikitext side of things I ended up going with option two and starting to write a generateWikiText function to encapsulate all of the wikitext functionality.
For clarification we have three cases:

1. fresh file -- what do we replace the "derived from" in the source field / what do we populate the source with? Maybe we could make a created by template for VCT?
2. overwriting an existing file -- edit previous wikitext's date and author. Source field remains the same.
3. saving existing file as new -- we are happy with the live version. source field gets "derived from" which points to the parent file.

Hey @Nicolasmichel
did you try with option 1 on wikiText side of things, if that api works without us sending the wikiText all together (does it use the previous one, or does it overwrites it with empty data as sent from api)

coming to your cases

1. i think as a default value we do have "Created and Uploaded by VideoCutTool" (its defined within that text field in Results.jsx)
2. source remains same yes, we will change date and author (do make sure to populate these fields from the api)
3. this flow remains same

so i think in current scenario both 1. and 3. are working expected

you can test it also by uploading couple of videos and checking their page (although make sure to remove them once testing is done)

So it seems like we dont have a default value for the source source field on the current live version.
The "Created/rotated/trimmed/cropped and Uploaded by VideoCutTool" you refer to above is the comment which is added as part of the description.
When looking at other files on commons, people either list an external url or "own work" in the source field.
It would not be difficult to add another text input box (similar to title) where we prompt the user for their source -- obviously only when uploading a fresh file, in cases 2 & 3 nothing from what is mentioned above would need to change.
As it stands the user could manually edit the wikitext to set their source but its not indicated that they need to at all.

As for the API, I verified that if we pass an empty text field, the previous text is kept (comment, source, author, & date), so if we want to change any of those values (for case 2) as I understand it we would want to grab the current text, edit the fields and then pass the whole thing back to api/upload.

In T338465#11380816, @Nicolasmichel wrote:

So it seems like we dont have a default value for the source source field on the current live version.
The "Created/rotated/trimmed/cropped and Uploaded by VideoCutTool" you refer to above is the comment which is added as part of the description.
When looking at other files on commons, people either list an external url or "own work" in the source field.
It would not be difficult to add another text input box (similar to title) where we prompt the user for their source -- obviously only when uploading a fresh file, in cases 2 & 3 nothing from what is mentioned above would need to change.
As it stands the user could manually edit the wikitext to set their source but its not indicated that they need to at all.

As for the API, I verified that if we pass an empty text field, the previous text is kept (comment, source, author, & date), so if we want to change any of those values (for case 2) as I understand it we would want to grab the current text, edit the fields and then pass the whole thing back to api/upload.

yes so for 2. we shouldn't send the wikiText at all in the first place, since we're overwriting on the existing video

for 3rd we should fetch it via api, and populate the field with it (unless user wants to change it, he/she can via clicking on pencil icon) [i think this functionality is already in place]

for 1. we can define the boilerplate wikiText (which I believe should be there, as default values)

now as far as source is concerned, i think its a part of wikiText itself, and so it would follow the same procedure as other fields within the wikiText are following

like for 1. we can set source same as that comment i mentioned (Edited and uploaded via vct by <username>)
this can be the source, description and comment (for now) [we can revisit this based on user response to it]

for 2. and 3. i feel source would be coming via the api itself (if not, then we also wouldn't be adding it via our side)

so basically the only difference b/w 2. and 3. case is, we allow user to edit the fields on 3. (not on 2.)

as far as a new input text field is concerned, yes we can have that, but wouldn't it add a extra constraint/code to parse the source from the wikiText, render it to that input, and if any change made on it, then we would have to reflect it on the api payload (before making the /api/upload call)

i feel for now, for the scope of this ticket, we can keep source within the wikiText textarea itself.
maybe we can have another ticket to move out the fields from wikiText and convert them into user friendly inputs

Heyo, I pushed a patch to the branch.
As a summary of the changes:
For fresh uploads:

1. Users uploading a fresh file are no longer presented with an overwrite option
2. Users can edit title, comment, and wikitext. Source and description fields in wikitext is set to the comment / whatever the user manually changes them to.

When editing an existing commons file (overwrite):

1. User can edit the comment. The title, and wikitext are un-editable.
2. API is passed an empty text field resulting in the overwrite keeping the original wikitext. The new comment can still be viewed in the file history section on commons. See example here.
3. Wikitext summary in VCT is not rendered. Even in live, we dont retrieve the whole wikitext of the video on commons, we retrieve the "building blocks" (author, comment, categories, title, url) and generate our expected wikitext off of those. As such, since we arent editing the wikitext in this case, we dont have anything to populate the summary with, so I prevented its rendering altogether when overwrite is ticked.

When editing an existing commons file (save as new file):

1. User can edit the comment, title, and wikitext.
2. Wikitext defaults to the derived-from template which points to the parent file. See example here.

Sorry about the multiple commits, I had moved some elements in the UI to see if it felt "better" and ended up bringing them back in line with the live version.
As usual, thanks in advance for the review.

In T338465#11385934, @Nicolasmichel wrote:

Heyo, I pushed a patch to the branch.
As a summary of the changes:
For fresh uploads:

1. Users uploading a fresh file are no longer presented with an overwrite option
2. Users can edit title, comment, and wikitext. Source and description fields in wikitext is set to the comment / whatever the user manually changes them to.

When editing an existing commons file (overwrite):

1. User can edit the comment. The title, and wikitext are un-editable.
2. API is passed an empty text field resulting in the overwrite keeping the original wikitext. The new comment can still be viewed in the file history section on commons. See example here.
3. Wikitext summary in VCT is not rendered. Even in live, we dont retrieve the whole wikitext of the video on commons, we retrieve the "building blocks" (author, comment, categories, title, url) and generate our expected wikitext off of those. As such, since we arent editing the wikitext in this case, we dont have anything to populate the summary with, so I prevented its rendering altogether when overwrite is ticked.

When editing an existing commons file (save as new file):

1. User can edit the comment, title, and wikitext.
2. Wikitext defaults to the derived-from template which points to the parent file. See example here.

Sorry about the multiple commits, I had moved some elements in the UI to see if it felt "better" and ended up bringing them back in line with the live version.
As usual, thanks in advance for the review.

hey @Nicolasmichel, the changes (based on summary) sounds good to me. I'll review the MR, and let you know if there're any comments.

Also can we write some unit tests for the above mentioned change, so that the flow is tested and covered by tests

also will deploy your changes on beta-vct to test them out (since the changes roll back to master every hour) let me know once you're available, will try to deploy it during that time, so that we both can test the changes live on beta env (at same time)

once all testing and review is done.. I'll merge the MR
thanks for your contribution :-)

hey @Nicolasmichel have reviewed the MR and left some comments. Please check and revert
thanks

Hello @Reputation22

Patch pushed. I addressed some of the comments you left.
Implemented some basic storage of wikitext to not regenerate on every toggle.
Cleaned up WT generation function a bit (mainly removed the ugly duplicate code).
Fixed the download path typo & removed / commented out leftover debugging console.log(s).

looks good to me
have approved the MR. Thanks for your contributions ;-)

Please rebase the branch with current master, post that I'll start with release.

In T338465#11380896, @Reputation22 wrote:

In T338465#11380816, @Nicolasmichel wrote:

So it seems like we dont have a default value for the source source field on the current live version.
The "Created/rotated/trimmed/cropped and Uploaded by VideoCutTool" you refer to above is the comment which is added as part of the description.
When looking at other files on commons, people either list an external url or "own work" in the source field.
It would not be difficult to add another text input box (similar to title) where we prompt the user for their source -- obviously only when uploading a fresh file, in cases 2 & 3 nothing from what is mentioned above would need to change.
As it stands the user could manually edit the wikitext to set their source but its not indicated that they need to at all.

As for the API, I verified that if we pass an empty text field, the previous text is kept (comment, source, author, & date), so if we want to change any of those values (for case 2) as I understand it we would want to grab the current text, edit the fields and then pass the whole thing back to api/upload.

yes so for 2. we shouldn't send the wikiText at all in the first place, since we're overwriting on the existing video

for 3rd we should fetch it via api, and populate the field with it (unless user wants to change it, he/she can via clicking on pencil icon) [i think this functionality is already in place]

for 1. we can define the boilerplate wikiText (which I believe should be there, as default values)

now as far as source is concerned, i think its a part of wikiText itself, and so it would follow the same procedure as other fields within the wikiText are following

like for 1. we can set source same as that comment i mentioned (Edited and uploaded via vct by <username>)
this can be the source, description and comment (for now) [we can revisit this based on user response to it]

for 2. and 3. i feel source would be coming via the api itself (if not, then we also wouldn't be adding it via our side)

so basically the only difference b/w 2. and 3. case is, we allow user to edit the fields on 3. (not on 2.)

as far as a new input text field is concerned, yes we can have that, but wouldn't it add a extra constraint/code to parse the source from the wikiText, render it to that input, and if any change made on it, then we would have to reflect it on the api payload (before making the /api/upload call)

i feel for now, for the scope of this ticket, we can keep source within the wikiText textarea itself.
maybe we can have another ticket to move out the fields from wikiText and convert them into user friendly inputs

@Nicolasmichel