Skip to content

show file size of archived courses when unarchiving#3047

Open
Alex-Jordan wants to merge 8 commits into
openwebwork:WeBWorK-2.21from
Alex-Jordan:archive-sizes
Open

show file size of archived courses when unarchiving#3047
Alex-Jordan wants to merge 8 commits into
openwebwork:WeBWorK-2.21from
Alex-Jordan:archive-sizes

Conversation

@Alex-Jordan

Copy link
Copy Markdown
Contributor

When at the Unarchive courses page, the list of courses now leaves off the ".tar.gz" and includes the size of the file to be unarchived. This will help me identify inefficiencies with how RS users are handling their courses. If I am about to unarchive a particularly large course, I will then go into it and see what can be trimmed. Usually, the faculty have put course archive files in their course. Or they are using images inefficiently. Or they copied entire folder trees for backup before editing local pg files.

I did this in a way to make it extensible. Perhaps there would be interest in also showing the date the archive file was last modified. Perhaps this is of tangential interest to @drdrew42 with #3043 in that the displayed course name could be the actual internal course name, not the file name with .tar.gz truncated off.

@Alex-Jordan

Copy link
Copy Markdown
Contributor Author

This now has a few styling changes too, in separate commits.

@drgrice1 drgrice1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be better to just update the listArchivedCourses method (instead of adding the statArchivedCourses method), and use the same format for showing archive files in the UI in the two places that is done (and the only two places where the listArchivedCourses method is used before this pull request). The two places are when unarchiving courses and on the main course admin page that lists courses and archived courses.

Comment thread lib/WeBWorK/Utils/CourseManagement.pm Outdated
@drgrice1

drgrice1 commented Jul 9, 2026

Copy link
Copy Markdown
Member

Part of the reason that I suggest just updating the listArchivedCourses method instead of adding the statArchivedCourses method is due to the messy structure of the created statArchivedCourses method. It first calls listArchivedCourses. That constructs a Mojo::File object representing the archive directory, lists all files in that directory with the .tar.gz extension, and returns the basename of all of those files. Then the statArchivedCourses method again creates a Mojo::File object representing the archive directory. Preprends it as a pure string (making it completely useless to construct the Mojo::File object) to the deconstructed Mojo::File basename of the listed archive files from the listArchiveCourses method. Note that if you have a Mojo::File object representing a directory, then you can get a Mojo::File object representing anything in that directory with the child method, and you can stat a file using the stat method.

If the two methods were combined much of that inefficiency and code clutter could be cleaned up. You just have to update lines 68-70 of templates/ContentGenerator/CourseAdmin.html.ep to use the updated return value.

Actually, I have put in a pull request to this branch that does the things I have mentioned.

drgrice1 and others added 2 commits July 9, 2026 06:33
The `listArchiveCourses` also stats the archive files found and returns
the files as a hash whose keys are the course ids and the values are
references to hashes containing the `filename` (the basename of the file
including the `.tar.gz` extension) and file `size`

Both of the places that use the `listArchiveCourses` method are updated
to use this return value.
Combine the `listArchiveCourses` and `statArchiveCourses` methods.
@Alex-Jordan

Copy link
Copy Markdown
Contributor Author

Considering #3043, what are your thoughts on making the keys be the actual course ID from inside each course archive file? For example (referencing the discussion in #3043) having drew => {filename => 'alex.tar.gz'}?

@drgrice1

drgrice1 commented Jul 9, 2026

Copy link
Copy Markdown
Member

Yeah, that would probably be a good idea considering the change in #3043. You can use basically the same Tar::Archive code @drdrew42 uses in that pull request to get that from the tar ball. Of course, not extracting the tar ball after it though.

@Alex-Jordan

Copy link
Copy Markdown
Contributor Author

This now has @drdrew's code from #3043 that gets the internal top folder name for the course ID instead of the file's name.

You can rename an archive file, and see the right thing at the Unarchive page. However you still can't actually unarchive such a file until #3043 is merged too.

@drgrice1 drgrice1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There are a few changes to the latest additions needed.

Comment thread lib/WeBWorK/Utils/CourseManagement.pm Outdated
Comment thread lib/WeBWorK/Utils/CourseManagement.pm Outdated
Comment thread lib/WeBWorK/Utils/CourseManagement.pm Outdated
@Alex-Jordan

Copy link
Copy Markdown
Contributor Author

Thanks for those notes. I applied them and squashed them in with the commit that moved to using the tarball's top folder.

@drgrice1 drgrice1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks good. I will give my approval, but note that this should not be merged until after #3043.

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.

2 participants