Add Fullname Feature to User Profiles by annapurna-gupta · Pull Request #2575 · Open-MSS/MSS

annapurna-gupta · 2024-11-24T17:56:09Z

##Fixes #1282

ReimarBauer

Hi, thanks for working on this!

There are some flake8 and linter failures found by the tests

Also you added some files which we don't need in the repository
remove

.coverage
dist/mss-9.2.0-py3.11.egg
instance/mscolab.db
output_file.py
pytest.log

There are multiple times added ui_add_user.py, ui_mscolab_connect_dialog.py, ui_mscolab_profile_dialog.py

I have to do a closer look on the other changes,

ReimarBauer · 2024-11-25T08:42:56Z

mslib/mscolab/server.py

+        user_record.nickname = nickname  # Update nickname
+
+        # Commit changes to the database
+        db.session.commit()


all these db commands needs to be moved to the file_manager

likly you can use modify_user
https://github.com/Open-MSS/MSS/blob/develop/mslib/mscolab/file_manager.py#L222

So I need to move all the additional commands to file_manager, and I should revert this part to how it was before.

not mandatory. check if you can use file_manager.modify_user you can maybe reuse the existing method

ReimarBauer · 2024-11-25T08:45:49Z

mslib/mscolab/server.py

+
+    except Exception as e:
+        # Log the error message (use logging instead of print for production)
+        print(f"Error updating user info: {str(e)}")  # Replace with proper logging if needed


when you need output, you have to use logging.debug, a print can create a traceback on a webserver

ReimarBauer · 2024-11-25T08:47:00Z

mslib/msui/mscolab.py

                self.logout()

+    def editfull_name(self):
+        if verify_user_token(self.mscolab_server_url, self.token):


there is a newer syntax for this by a decorator. Have a look on the other lines where that is used

ReimarBauer · 2024-11-25T08:48:09Z

mslib/msui/qt5/ui_add_user_dialog.py

        self.rePassword.setPlaceholderText(_translate("addUserDialog", "Confirm your password"))
-        self.emailIDLabel.setText(_translate("addUserDialog", "Email:"))
-        self.usernameLabel.setText(_translate("addUserDialog", "Username:"))
+


this section is not needed, likly this comes from an option of the pyuic5 command.

do you mean all the changes in ui_add_user_dialog.py is not needed??

Add only the fullname

ReimarBauer · 2024-11-25T08:48:36Z

mslib/msui/qt5/ui_mscolab_profile_dialog.py



-from . import resources_rc
+if __name__ == "__main__":


this is also not needed

ReimarBauer · 2024-11-25T08:49:28Z

ui_add_user.py

@@ -0,0 +1,10 @@
+# -*- coding: utf-8 -*-


file on wrong place?

i removed it.

ReimarBauer · 2024-11-25T08:49:37Z

ui_mscolab_connect_dialog.py

@@ -0,0 +1,10 @@
+# -*- coding: utf-8 -*-


file on wrong place?

i removed it

ReimarBauer · 2024-11-25T08:49:45Z

ui_mscolab_profile_dialog.py

@@ -0,0 +1,10 @@
+# -*- coding: utf-8 -*-


file on wrong place?

ReimarBauer · 2024-11-25T08:50:38Z

.coverage

we don't need this in the repository

okk i have removed it..

ReimarBauer · 2024-11-25T08:50:52Z

dist/mss-9.2.0-py3.11.egg

we don't need this in the repository

okk i have removed it..

ReimarBauer · 2024-11-25T08:51:03Z

instance/mscolab.db

we don't need this in the repository

okk i have removed it..

mslib/mscolab/models.py

ReimarBauer · 2024-11-26T09:12:43Z

I think we should keep creating an useraccount as it is now, and add into the profile dialog the feature to set a fullname and a nickname.

We need to be able to edit it from here anyway.

matrss · 2024-11-26T10:31:59Z

I won't be able to fully review this until next week, but just a quick comment: we already have a username field, and I understand usernames and nicknames as being the same thing. So I don't see a reason to have two nickname fields when we could just make the username changeable (if it isn't already).

ReimarBauer

see comments

ReimarBauer · 2024-11-26T08:55:15Z

mslib/msui/qt5/ui_mscolab_profile_dialog.py

+        self.label_5.setText(_translate("ProfileWindow", ":"))
        self.deleteAccountBtn.setText(_translate("ProfileWindow", "Delete Account"))
        self.uploadImageBtn.setText(_translate("ProfileWindow", "Change Avatar"))
+import resources_rc


where does this comes from?

likly you need to convert the ui file with
pyuic5 --from-imports ui_mscolab_profile_dialog.ui > ../qt5/ui_mscolab_profile_dialog.py

what do you mean? i have converted .ui file to .py file .

ReimarBauer · 2024-11-28T15:10:37Z

mslib/mscolab/server.py

+        user_record.nickname = nickname  # Update nickname
+
+        # Commit changes to the database
+        db.session.commit()


not mandatory. check if you can use file_manager.modify_user you can maybe reuse the existing method

mslib/mscolab/models.py

ReimarBauer · 2024-11-29T08:08:39Z

mslib/msui/qt5/ui_add_user_dialog.py

        self.rePassword.setPlaceholderText(_translate("addUserDialog", "Confirm your password"))
-        self.emailIDLabel.setText(_translate("addUserDialog", "Email:"))
-        self.usernameLabel.setText(_translate("addUserDialog", "Username:"))
+


Add only the fullname

ReimarBauer · 2024-11-29T08:09:28Z

mslib/msui/qt5/ui_mscolab_connect_dialog.py

        self.idpAuthTopicLabel.setText(_translate("MSColabConnectDialog", "Identity Provider Authentication"))
        self.statusLabel.setText(_translate("MSColabConnectDialog", "Status:"))
+
+


the __name__ section gets in by some conversion option, we don't need that

mslib/index.py

ReimarBauer · 2024-12-06T07:52:19Z

mslib/index.py


    APP.jinja_env.globals.update(get_topmenu=get_topmenu)

-    @APP.route("/index")


have you moved this? or why is it deleted?

http://localhost:8083/ seems broken.

mslib/mscolab/migrations/versions/a63201ddec7a_to_version_next_major_version.py

ReimarBauer · 2024-12-06T07:54:06Z

output_file.py

@@ -0,0 +1,10 @@
+# -*- coding: utf-8 -*-


why do we need this?

pytest.log

ReimarBauer · 2024-12-06T07:56:27Z

mslib/msui/mscolab.py

+            self.ui,
+            self.ui.tr("Edit Full Name"),
+            self.ui.tr(
+                f"You're about to change the full name - '{self.active_operation_name}' "


What is the reason to add here self.active_operation_name ? How is the users full name linked to an operation?

i am confused, I need to collect the user's full name and nickname from the UI, send the data to the server, and update the UI with the response from the server. Is this the correct approach?

Start with only adding the full name. See discussion by Matthias.

My question is why is the users fullname related to the operation name?

Screenshot_20241207_041506_GitHub.jpg

sorry.. it's not directly related.

Start with only adding the full name. See discussion by Matthias.

where can i find it?

#2575 (comment)

ReimarBauer · 2024-12-06T07:57:46Z

mslib/msui/mscolab.py

+
+
+    def editnick_name(self):
+        pass


This is currently a stub, or?

this is not needed, or?

ReimarBauer

see comments/questions

ReimarBauer

the migration script seems not correct

Are you sure you want to reset the database? This would delete all your data! (y/[n]):y
INFO [alembic.runtime.migration] Context impl SQLiteImpl.
INFO [alembic.runtime.migration] Will assume non-transactional DDL.
INFO [alembic.runtime.migration] Running downgrade 922e4d9c94e2 -> c171019fe3ee, To version 10.0.0
Traceback (most recent call last):
...
KeyError: 'fullname'

you get on this by

annapurna-gupta/MSS$ python mslib/mscolab/mscolab.py db --reset

please also undo your changes to index.py. This is our homepage.

you can see the crash by

annapurna-gupta/MSS$ python mslib/mswms/mswms.py
on your browser http://127.0.0.1:8081

ReimarBauer · 2024-12-30T11:52:47Z

seems I am not allowed to propose changes to your PR, so that is as patch here

diff --git a/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py b/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py
index 35fd014c..21c353c2 100644
--- a/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py
+++ b/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py
@@ -21,8 +21,6 @@ def upgrade():
     # ### commands auto generated by Alembic - please adjust! ###
     with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.add_column(sa.Column('profile_image_path', sa.String(length=255), nullable=True))
-    
-    with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.add_column(sa.Column('fullname', sa.String(length=255), nullable=True))
 
     # ### end Alembic commands ###
@@ -32,8 +30,6 @@ def downgrade():
     # ### commands auto generated by Alembic - please adjust! ###
     with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.drop_column('profile_image_path')
-
-    with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.drop_column('fullname')
 
      # ### end Alembic commands ###

annapurna-gupta · 2025-01-02T14:17:19Z

the migration script seems not correct

Are you sure you want to reset the database? This would delete all your data! (y/[n]):y INFO [alembic.runtime.migration] Context impl SQLiteImpl. INFO [alembic.runtime.migration] Will assume non-transactional DDL. INFO [alembic.runtime.migration] Running downgrade 922e4d9c94e2 -> c171019fe3ee, To version 10.0.0 Traceback (most recent call last): ... KeyError: 'fullname'

you get on this by
annapurna-gupta/MSS$ python mslib/mscolab/mscolab.py db --reset
please also undo your changes to index.py. This is our homepage.

you can see the crash by
annapurna-gupta/MSS$ python mslib/mswms/mswms.py
on your browser http://127.0.0.1:8081

so should i change migration script by using that command? and I have undone the changes to index.py.

annapurna-gupta · 2025-01-02T14:28:09Z

seems I am not allowed to propose changes to your PR, so that is as patch here

diff --git a/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py b/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py
index 35fd014c..21c353c2 100644
--- a/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py
+++ b/mslib/mscolab/migrations/versions/922e4d9c94e2_to_version_10_0_0.py
@@ -21,8 +21,6 @@ def upgrade():
     # ### commands auto generated by Alembic - please adjust! ###
     with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.add_column(sa.Column('profile_image_path', sa.String(length=255), nullable=True))
-    
-    with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.add_column(sa.Column('fullname', sa.String(length=255), nullable=True))
 
     # ### end Alembic commands ###
@@ -32,8 +30,6 @@ def downgrade():
     # ### commands auto generated by Alembic - please adjust! ###
     with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.drop_column('profile_image_path')
-
-    with op.batch_alter_table('users', schema=None) as batch_op:
         batch_op.drop_column('fullname')
 
      # ### end Alembic commands ###

but the checkbox labeled 'Allow edits from maintainers' is checked.

mslib/mscolab/migrations/versions/92eaba86a92e_to_version_8_3_5_initial_migration.py

mslib/mscolab/file_manager.py

ReimarBauer · 2025-01-22T09:26:20Z

mslib/mscolab/server.py

+        user_record.fullname = fullname  # Update full name
+
+        # Commit changes to the database
+        _handle_db_upgrade().session.commit()


this needs to use the file_manager

mslib/mscolab/file_manager.py

ReimarBauer · 2025-01-22T12:42:51Z

mslib/msui/qt5/ui_mscolab_profile_dialog.py

        self.uploadImageBtn.setText(_translate("ProfileWindow", "Change Avatar"))
-
-
-from . import resources_rc


that is an option of pyuic5 --from-imports

ReimarBauer · 2025-01-22T15:29:25Z

I am still not able to push to your changes/repository:
Fehler: Fehler beim Versenden einiger Referenzen nach 'github.com:annapurna-gupta/MSS.git'

please apply the patch by the patch command, patch -p1 < changes_needed.patch.txt

     -pnum  or  --strip=num
          Strip the smallest prefix containing num leading slashes from each file name found in the patch file.  A sequence of one or more adjacent slashes is counted as a single slash.   This
          controls  how file names found in the patch file are treated, in case you keep your files in a different directory than the person who sent out the patch.  For example, supposing the
          file name in the patch file was

changes_needed.patch.txt

annapurna-gupta · 2025-01-22T19:10:19Z

patch -p1 < changes_needed.patch.txt

I tried applying the patch, but I can't find the patch file. Could you please share the file or its location so I can proceed?

ReimarBauer · 2025-01-22T20:44:52Z

It is the blue word above your Posting.
I attached it as txt file. And when you click on it you can download it.

Just tested

annapurna-gupta · 2025-01-23T13:45:25Z

It is the blue word above your Posting. I attached it as txt file. And when you click on it you can download it.

Just tested

i found it, thank you.

ReimarBauer · 2025-01-24T07:29:20Z

mslib/mscolab/server.py

    return jsonify({"success": True, "message": "Successfully set version name"})


+@APP.route("/edit_user_info", methods=["POST"])


where is this used?

When it is planned for

The function should replace the Change Avatar, by an Edit.

e-mail won't be allowed to become changed

and for changing the username you need to verify that the new one was not taken.

The new Edit should also be possible to edit the Avatar.

mslib/mscolab/models.py

mslib/mscolab/seed.py

Add immutable fullname and nickname fields to user profiles

4e9f830

ReimarBauer requested changes Nov 25, 2024

View reviewed changes

ReimarBauer reviewed Nov 25, 2024

View reviewed changes

mslib/mscolab/models.py Show resolved Hide resolved

ReimarBauer requested a review from matrss November 25, 2024 09:30

ReimarBauer reviewed Nov 29, 2024

View reviewed changes

Updated fullname and nickname fields

2716bd3