Open-MSS / MSS

A QT application, a OGC web map server, a collaboration server to plan atmospheric research flights.
https://open-mss.github.io
Apache License 2.0
54 stars 68 forks source link

feat: Allow users to set custom profile pictures #2405

Open workaryangupta opened 2 weeks ago

workaryangupta commented 2 weeks ago

This pull request aims to introduce a new feature to mscolab, enabling users to upload and set personalized profile pictures.

Screenshot (292)

matrss commented 2 weeks ago

Please take a look at the CI test failures. You have introduced a new required positional argument to a constructor that is not supplied everywhere it is used, so it crashes a lot of tests.

I think the profile image should be optional and the default behavior should match what we had before this PR.

workaryangupta commented 2 weeks ago

I made the profile image optional but still, the tests are failing. There is something more to it that I am not able to grasp.

matrss commented 2 weeks ago

This is now a new test failure because some data files in skyfield-data have expired yesterday. This is unrelated to your code. Coincidentally, I've already noticed this last week and opened an issue: https://github.com/brunobord/skyfield-data/issues/36.

@ReimarBauer it looks like you fixed this the last time it came up, judging by the commit history in skyfield-data. Do you mind taking a look again? I am not quite sure what the process is, over there.

ReimarBauer commented 2 weeks ago

You could xfail the test with a good hint to our issue, so it can be later enabled again. https://github.com/Open-MSS/MSS/issues/2406

ReimarBauer commented 9 hours ago

there are failing tests because of the changes

FAILED tests/_test_mscolab/test_sockets_manager.py::Test_Socket_Manager::test_upload_file - AssertionError: assert 'png' in 'mss-logo-20240702T145239-gg_WLzWnSYww0LGwa8mIRQ.unknown'

There the extension was removed. We want to know the extension.

FAILED tests/_test_mscolab/test_chat_manager.py::Test_Chat_Manager::test_add_attachment - AttributeError: 'ChatManager' object has no attribute 'add_attachment'

There the method was changed?

workaryangupta commented 9 hours ago

FAILED tests/_test_mscolab/test_chat_manager.py::Test_Chat_Manager::test_add_attachment - AttributeError: 'ChatManager' object has no attribute 'add_attachment'

There the method was changed?

add_attachment method was not needed, so I just removed it. Should I remove the test for it too since the method no longer exists?

workaryangupta commented 6 hours ago

@matrss could you also guide me about resolving these test failures? https://github.com/Open-MSS/MSS/actions/runs/9763173464/job/26948403618?pr=2405#step:10:2726 https://github.com/Open-MSS/MSS/actions/runs/9763173464/job/26948403618?pr=2405#step:10:2727

matrss commented 5 hours ago

test_add_attachment obviously fails because there is no add_attachment method anymore. This test could/should be moved to the other tests for the FileManager, where the replacement of add_attachment now resides, renamed and adapted to test upload_file instead.

test_upload_file fails because the upload_file tries to take the mimetype from file, but that mimetype is empty for some reason. I think this is a bug in the test because it doesn't specify the content type of the uploaded file in its post request.

matrss commented 4 hours ago

According to this: https://stackoverflow.com/a/51384608 flask doesn't do the right thing anyway to determine the mime type (it also just guesses from the file name and trusts the client to provide something correct). I don't want to introduce an additional dependency on python-magic though.

You could just leave out the file extension entirely, after all they aren't required on any sane system. Again, this would require a fix of test_upload_file though, since it currently enforces an extension for some reason.