Skip to content

Conversation

@tcnichol
Copy link
Contributor

@tcnichol tcnichol commented Aug 8, 2023

Right now we can change the file version.

The file summary has not changed to reflect the old version information yet. That will be done after metadata. Only the name of the left component changes.

I am now sending the selected version to the file metadata, but I am not sure that this is providing the correct version's metadata. Will need to start over with a new file with new metadata to test.

Another thing that does not work. If you have the metadata tab open, and then you change the file version using the modal, it does not change to reflect the new file version. I was not sure how to handle this.

@tcnichol tcnichol linked an issue Aug 8, 2023 that may be closed by this pull request
@tcnichol
Copy link
Contributor Author

tcnichol commented Aug 9, 2023

The following now works:
Changing file version will change the ListenerMetadata and the File Details is replaced with File History, where bytes, created, author are replaced with values from the previous version.

Right now there is no change for visualization. Will make that a separate issue.

To test, upload a text file, run wordcount extractor, and then modify it to create some new versions and re-run extractor. When you change, you will see the metadata change and also, the file details will change.

@tcnichol tcnichol requested a review from lmarini August 10, 2023 03:34
Copy link
Contributor

@max-zilla max-zilla left a comment

Choose a reason for hiding this comment

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

There is a bug where I uploaded Squirrel.gif, then tried to upload a new file version using OtherFile.gif. The API does not allow a new file version to have different filename from before, so it returns a 400, but the UI does not seem to catch this and tries to show the newer version of the file (that didn't successfully get created). Not sure if this bug was introduced here but we should fix it here. (see routers/files update_file method for this logic - we should catch the 400)
image

main_type: str = "N/A"


class FileVersionDetails(BaseModel):
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not used, please remove

export const CHANGE_SELECTED_VERSION = "CHANGE_SELECTED_VERSION";

export function changeSelectedVersion(fileId, selectedVersion) {
console.log('change selected version in actions', fileId, selectedVersion);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove

const fileVersions = useSelector(
(state: RootState) => state.file.fileVersions
);
console.log(fileVersions[0], 'the fisrt');
Copy link
Contributor

Choose a reason for hiding this comment

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

remove

@tcnichol
Copy link
Contributor Author

Debugging with the file update, I'm seeing now that the right error message does pop up first:
correct_error
Then it looks like the wrong one comes after that. Trying to figure out what is going on.

@max-zilla max-zilla self-requested a review August 24, 2023 13:39
Copy link
Contributor

@max-zilla max-zilla left a comment

Choose a reason for hiding this comment

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

tested uploading a file and triggering a rejection on the update, switching versions, everything seemed to work.

@max-zilla
Copy link
Contributor

@tcnichol i think if you resolve merge conflict we can merge

@lmarini
Copy link
Member

lmarini commented Aug 24, 2023

To do in this or future PR:

  • make button a pull down
  • remove change version buttons (next to each version)

@ddey2 ddey2 self-requested a review August 24, 2023 16:06
@tcnichol
Copy link
Contributor Author

Now the file version should be next to and not underneath the filename in the files table.
Screenshot 2023-08-24 at 3 58 37 PM

@lmarini lmarini merged commit ed7a6c0 into main Aug 31, 2023
@lmarini lmarini deleted the 620-view-previous-file-version-metadata branch August 31, 2023 14:35
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.

view previous file version metadata and file details

5 participants