Skip to content

Show both the course id and the archive file for duplicates when listing course archives in the admin course. - #3114

Closed
drgrice1 wants to merge 2 commits into
openwebwork:developfrom
drgrice1:course-archive-list-filename
Closed

Show both the course id and the archive file for duplicates when listing course archives in the admin course.#3114
drgrice1 wants to merge 2 commits into
openwebwork:developfrom
drgrice1:course-archive-list-filename

Conversation

@drgrice1

@drgrice1 drgrice1 commented Aug 5, 2026

Copy link
Copy Markdown
Member

The previous setup did not take into account what would happen if multiple archive files contained the same course id. So instead of the listArchivedCourse method returning a hash of the form

{
    myTestCourse => {
        filename => 'myTestCourse.tar.gz',
        size     => '605 KB'
    }
}

it now returns a hash of the form

{
    'myTestCourse.tar.gz' => {
        courseID     => 'myTestCourse',
        size         => '605 KB',
        lastModified => 1778667472
    }
}

Note that the lastModified key is only included because that is what is saved in the cache file, and that is just directly returned if the archive file has not been modified. Although, at this point it is not used elsewhere.

In addition when duplicate courseIDs are seen each of those in the hash will contain the duplicateCourseID key.

Then both the courseID and the filename are then displayed for those archives that are duplicate courseIDs. The format displayed is

notDuplicatedCourseID (2 GB)
myTestCourse (myTestCourse.tar.gz, 605 KB)
myTestCourse (myTestCourseAlt.tar.gz, 1.6 MB)

…ives in the admin course.

The previous setup did not take into account what would happen if
multiple archive files contained the same course id.  So instead of the
`listArchivedCourse` method returning a hash of the form

```perl
{
    myTestCourse => {
        filename => 'myTestCourse.tar.gz',
        size     => '605 KB'
    }
}
```

it now returns a hash of the form

```perl
{
    'myTestCourse.tar.gz' => {
        courseID     => 'myTestCourse',
        size         => '605 KB',
  	lastModified => 1778667472
    }
}
```

Note that the `lastModified` key is only included because that is what
is saved in the cache file, and that is just directly returned if the
archive file has not been modified. Although, at this point it is not
used elsewhere.

Both the courseID and the filename are then displayed.  The format
displayed is `myTestCourse (myTestCourse.tar.gz, 605 KB)`.

If there is also an archive file `myTestCourseAlt.tar.gz` then it will
also be listed as something like`myTestCourse (myTestCourseAlt.tar.gz, 1.6 MB)`.
@drgrice1

drgrice1 commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Note that the first commit in this pull request always shows the filename for all archives, and does not have the duplicateCourseID key handling. The second commit adds that. I am not sold on only showing the filename for duplicates, and the second commit could be removed to go back to always showing the archive filenames.

@drgrice1
drgrice1 force-pushed the course-archive-list-filename branch from 989c441 to 7cce56a Compare August 5, 2026 23:32
@drgrice1
drgrice1 force-pushed the course-archive-list-filename branch from 7cce56a to 12c556a Compare August 5, 2026 23:39
@Alex-Jordan

Copy link
Copy Markdown
Contributor

I lean toward only printing the file name when there is a difference, for these reasons:

  • think that most servers will have only files that match the course ID. So showing the file in addition to the course ID is only adding clutter to most servers.
  • When there are mismatches, this would draw attention to the mismatches and help the admin understand which files have a mismatch.

I don't mind if it goes the other way though. If I care later, I can just apply your second commit to my instances.

@somiaj

somiaj commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

I think it is only showing file names of duplicated courseIDs, but I agree it should also show file names if the file name is different than the course ID. Besides for the reasons listed by @Alex-Jordan, it will also help if the admin goes to the file manager looking for a course name1 but it isn't in the list because it was listed under name2.

I don't have strong opinions on if they are always shown (I do like the idea of code simplification) or not, though in most cases if files aren't being renamed or no duplicates, the information is not needed, so I slightly prefer only showing the filename in the case it is different/duplicates exist.

@drgrice1

drgrice1 commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

So only showing the filename of it differs from the course id is a different way to go. It would be simpler to implement.

@drgrice1

drgrice1 commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

What is the direction to go?

@Alex-Jordan

Copy link
Copy Markdown
Contributor

The list of active courses shows course IDs. So my instinct is that the list of archived courses should always show course IDs as well.

And then if that's a course ID that does not match its filename, show the filename. So like:

Courses

  1. apple
  2. banana

Archived Courses

  1. banana (1MB)
  2. cherry (2MB)
  3. daikon (1MB)
  4. daikon (endive.tar.gz 1MB)
  5. daikon (fig.tar.gz 2MB)

Again, this is just what I'm thinking. I'm OK with whatever in the end, now that you fixed that slowness issue.

By the way, now that I think about it, why are these ordered lists?

@drgrice1

drgrice1 commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

The list of archives already and still always shows the course id. These pull requests are about whether or not the archive filename is also shown.

So now we need a vote on this pull request (show both course id and filename for all archive files that share a course id), #3116 (show both course id and filename when the course id and filename are different), or drop the last commit in any of these pull requests and always show both the course id and archive filename.

@Alex-Jordan

Copy link
Copy Markdown
Contributor

Do I understand right that #3116 (with both commits) would make it like my example earlier? And this one, #3114 with both commits, would look almost the same except it would have 3. daikon (daikon.tar.gz 1MB)?

@Alex-Jordan

Copy link
Copy Markdown
Contributor

If so, I vote for #3116 with both commits.

@drgrice1

drgrice1 commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

As to these being ordered lists (I assume you mean on the "Course Listings" page, because on the "Unarchive Course" page they aren't) they have always been ordered lists. Would you rather they were unordered lists?

@somiaj

somiaj commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

This one would also only show cantaloupe (3 MB) even if the file name was different. The other would show cantaloupe (honeydew.tar.gz, 3 MB). I too would rather have #3116. I'll go put my approvals there.

@drgrice1

drgrice1 commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

With this pull request you would see the filename for all course archives for which another course archive contains the same course id.

With #3116 you see the filename for any course archive for which the filename and course id are different.

@Alex-Jordan

Copy link
Copy Markdown
Contributor

It just strikes me as odd to enumerate things that are already in an alphabetical order. Having bullets would be worse than numbers for this (imo) though. Let's forget I said anything about that.

@drgrice1

drgrice1 commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

In any case changing these from an ordered list to something else would be for another pull request and for the next release. I will forget you mentioned it.

@Alex-Jordan

Copy link
Copy Markdown
Contributor

Closing in favor of #3116.

@Alex-Jordan Alex-Jordan closed this Aug 6, 2026
@drgrice1
drgrice1 deleted the course-archive-list-filename branch August 6, 2026 10:21
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