id summary reporter owner description type status priority milestone component resolution keywords cc parents 927 Clean up Federation Jessica Tallon "Elrond found some issues over his review of the federation code recently merged. These need fixing before 0.7.0. '''Garbage Collection''' - This task should be moved to submit - This task uses the wrong operator in garbage collection (greater than rather than less then) - The test sets entry.uploaded not entry.created ergo doesn't do anything. - Logging is wrong. - Use spaces not tabs! (WTF jess). '''Feed Endpoint''' - The comment is wrong (should say list all media not the users media). '''Whoami Endpoint''' - Doesn't handle the case where I might not be logged in '''MediaComment.unserialize''' - Needs to do more checking that all the data I want to look up in the dictionary exists to avoid KeyErrors. '''Mediagoblin INI/Config file''' - Should remove garbage_collection setting to spec file and remove it from mediagoblin.ini as it won't be commonly set. '''json_error''' * On the idea of: {{{ def json_error(error_str, status=400): return json_response({""error"": error_str}, status=status) }}} * I am not sure, where to put this, either in the general response place, where json_response is also, or maybe only where it's being used in the federation code. If I have to vote, I'd likely vote for the former. '''media_type/image: api_* upload utils''' * Those staticmethod functions are really a specialized variant of submit/lib.py:submit_media(). * As only the pump.io/federation code uses it, please move this into federation/lib.py * In the long run, this should be refactored together with submit/lib.py to provide a consistent upload api for everybody, again. '''views.py security''' * oauth authenticates user A; the URL contains user B; And for some cases, there is an already existing object O, owned by user C. * I guess, that all three users must be the same. If not, please explain in detail why (comments in code!) * Testing for A == B is already in the code. * Testing for C == A is not yet there. '''view.py style''' * There are a bunch of {{{ obj = Klass.query.filter(...) if obj is None: return error obj = obj[0] }}} in there. The if will never fire, as filter() always returns an unfinished query object. Please replace by {{{ obj = ...filter(...).first() if obj is None: return error }}} If any more issues are found please add them to this ticket. This is a rather large issue but most of the tasks in them are so small it doesn't warrant splitting them up. " defect closed critical 0.7.0 programming fixed