id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
31517	ManifestFilesMixin.file_hash() returning None get's included in hashed filename as 'None'.	Richard Campen	Richard Campen	"When returning a string from a custom` ManifestFilesMixin.file_hash()` implementation, the resulting file name is `<file_path>.<custom_hash>.<ext>` as expected, whereas returning `None` results in `<file_path>None.<ext>`.

[https://groups.google.com/forum/#!searchin/django-developers/file_hash%7Csort:date/django-developers/GXNDGBb3tlM/VE61CAgoAgAJ Discussion on django-developers] supports this behaviour being unintended.

Behavior appears to have been introduced with #17896 which split the file hashing into a separate method.

The following test, when included in the `test_storage.TestCollectionManifestStorage` test class demonstrates the bug:


{{{
def test_hashed_name_unchanged_when_file_hash_is_None(self):
    with mock.patch('django.contrib.staticfiles.storage.ManifestStaticFilesStorage.file_hash', return_value=None):
        self.assertEqual(storage.staticfiles_storage.hashed_name('test/file.txt'), 'test/file.txt')
}}}


As suggested by the name of my test, my opinion is that the correct behaviour should be that if `file_hash` returns None, then no hash is inserted into the filename and it therefore remains unchanged.

With that in mind, a possible solution is to change the following  lines in the `hashed_name()` method (~line 100 in `staticfiles.storage`):

{{{
if file_hash is not None:
    file_hash = "".%s"" % file_hash
hashed_name = os.path.join(path, ""%s%s%s"" % (root, file_hash, ext))
}}}

to

{{{
if file_hash is None:
    file_hash = """"
else:
    file_hash = "".%s"" % file_hash
hashed_name = os.path.join(path, ""%s%s%s"" % (root, file_hash, ext))
}}}"	Cleanup/optimization	closed	contrib.staticfiles	dev	Normal	fixed	HashedFilesMixin, ManifestFilesMixin, file_hash, hashed_name		Ready for checkin	1	0	0	0	1	0