Description J Leadbetter 2023-07-06 09:15:30 CEST

## Background

This bug was discovered while looking into [Bug 56231](https://forge.univention.org/bugzilla/show_bug.cgi?id=56231). Bug #56231 was caused by Cherrypy mangling the file names during upload, and was resolved by moving to Tornado, which correctly handles the filenames.

However, the underlying problem still exists in the code, that if for some reason files aren't copied over to the project cachedir (or copied with the wrong name, because of encoding, per Bug #56231), the project immediately becomes "distributed" in the UI and it isn't possible to distribute the materials.


## How to Reproduce

If you want to reproduce this bug on 5.0-3, you can simply upload files with umlauts in the name. For 5.0-4, where the file name mangling problem is solved, you have to do this a lot more artificially in order to simulate upload issues.

1. On a UCS@school server in a browser, go to "Education -> Distribute Materials".
2. Create a new project. When uploading files, be sure to include some with umlauts in the name -- e.g. "fün-with-ümläuts.txt".

If you are reproducing on 5.0-4, you can simulate a file upload problem by doing the following:

1. SSH into the server where you created the project.
2. Go to `/var/lib/ucs-school-umc-distribution` and look for the `*.data` folder with your project name.
3. Delete or rename any of the files you uploaded.
4. Go back to the browser and refresh the page. The project will now be marked as distributed.


## Cause of the Bug

If you look at the info file in `/var/lib/ucs-school-umc-distribution` for the project, you can see that the project is still `"isDistributed": false`.

The problem comes from [this set of lines](https://git.knut.univention.de/univention/ucsschool/-/blob/89af3bc95d6cb0abd3138abc7e0f6742feb484d5/ucs-school-umc-distribution/umc/python/distribution/util.py#L831-L836) in the code:

```
            if not project.isDistributed:
                #  Projects created before bug #47160 was fixed didn't save their distribution status
                files = [
                    ifn for ifn in project.files if os.path.exists(os.path.join(project.cachedir, ifn))
                ]
                project.isDistributed = len(files) != len(project.files)
```

The logic is wrong here for two reasons:

1. The original fix for Bug #47160 is supposed to handle if `project.isDistributed` was missing, but this unfortunately also sets distribution to "True" in cases where `project.isDistributed` is set and is "False".
2. The fix for Bug #47160 is already handled correctly (it only sets the status when `isDistributed` is missing) in [jsonDecode](https://git.knut.univention.de/univention/ucsschool/-/blob/89af3bc95d6cb0abd3138abc7e0f6742feb484d5/ucs-school-umc-distribution/umc/python/distribution/util.py#L172-L176), which is called immediately prior to this code -- i.e., this is unnecessary duplicate code.

The recommended fix is just to remove the lines of code shown above. You can verify that this fixes the issue by going onto the server, removing the lines of code, and restarting the server. Refresh the "Distribute materials" page, and you'll see that the project goes back to being correctly identified as not distributed.


## Desired Fixes

During the investigation of Bug #56231, we discovered that the [tests that should have caught the problem](https://git.knut.univention.de/univention/ucsschool/-/blob/89af3bc95d6cb0abd3138abc7e0f6742feb484d5/ucs-test-ucsschool/90_ucsschool/18_distribute_materials_encoding.py) is being skipped in the Jenkins run. We should figure out why this is being skipped, so we can monitor for regressions specifically related to filenaname mangling.

Here are the proposed fixes:

1. Remove the code mentioned above.
2. Add a test that projects with `"isDistributed": false` will correctly display as not distributed, even if files are missing.
3. Re-enable the `18_distribute_materials_encoding.py` test so it runs with the nightly Jenkins test jobs.