-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Fullname and Nickname Feature to User Profiles #2575
base: develop
Are you sure you want to change the base?
Add Fullname and Nickname Feature to User Profiles #2575
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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,
user_record.nickname = nickname # Update nickname | ||
|
||
# Commit changes to the database | ||
db.session.commit() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I need to move all the additional commands to file_manager, and I should revert this part to how it was before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not mandatory. check if you can use file_manager.modify_user you can maybe reuse the existing method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okk
|
||
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when you need output, you have to use logging.debug, a print can create a traceback on a webserver
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okk
@@ -966,6 +966,34 @@ def delete_account(self, _=None): | |||
if r.status_code == 200 and json.loads(r.text)["success"] is True: | |||
self.logout() | |||
|
|||
def editfull_name(self): | |||
if verify_user_token(self.mscolab_server_url, self.token): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a newer syntax for this by a decorator. Have a look on the other lines where that is used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okkkk
self.emailid.setPlaceholderText(_translate("addUserDialog", "[email protected]")) | ||
self.passwordLabel.setText(_translate("addUserDialog", "Password:")) | ||
self.password.setPlaceholderText(_translate("addUserDialog", "Your password")) | ||
self.confirmPasswordLabel.setText(_translate("addUserDialog", "Confirm Password:")) | ||
self.rePassword.setPlaceholderText(_translate("addUserDialog", "Confirm your password")) | ||
self.emailIDLabel.setText(_translate("addUserDialog", "Email:")) | ||
self.usernameLabel.setText(_translate("addUserDialog", "Username:")) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this section is not needed, likly this comes from an option of the pyuic5 command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you mean all the changes in ui_add_user_dialog.py is not needed??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add only the fullname
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
|
||
|
||
from . import resources_rc | ||
if __name__ == "__main__": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is also not needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okkk
@@ -0,0 +1,10 @@ | |||
# -*- coding: utf-8 -*- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file on wrong place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i removed it.
@@ -0,0 +1,10 @@ | |||
# -*- coding: utf-8 -*- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file on wrong place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i removed it
@@ -0,0 +1,10 @@ | |||
# -*- coding: utf-8 -*- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file on wrong place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need this in the repository
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okk i have removed it..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need this in the repository
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okk i have removed it..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need this in the repository
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okk i have removed it..
@@ -65,8 +65,11 @@ class User(db.Model): | |||
permissions = db.relationship('Permission', cascade='all,delete,delete-orphan', backref='user') | |||
authentication_backend = db.Column(db.String(255), nullable=False, default='local') | |||
|
|||
fullname = db.Column(db.String(255), nullable=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because you change the dbase model, you need to do additional steps, see
https://github.com/Open-MSS/MSS/blob/develop/docs/development.rst#changing-the-database-model
when that is not done, a annapurna-gupta/MSS$ python mslib/mscolab/mscolab.py db --seed
crashes with
sqlite3.OperationalError: table users has no column named fullname
also it may be useful to change the seed.py to add some of this data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i have generated the migration script but while applying it to database i am facing import error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please show the failure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please show your PYTHONPATH
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
likly PYTHONPATH should show D:\LOCAL_CODE COPY\MSS
I think on windows you can use setx PYTHONPATH D:\LOCAL_CODE COPY\MSS
currently python can't find mslib. ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comments
self.deleteAccountBtn.setText(_translate("ProfileWindow", "Delete Account")) | ||
self.uploadImageBtn.setText(_translate("ProfileWindow", "Change Avatar")) | ||
import resources_rc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where does this comes from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
likly you need to convert the ui file with
pyuic5 --from-imports ui_mscolab_profile_dialog.ui > ../qt5/ui_mscolab_profile_dialog.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean? i have converted .ui file to .py file .
user_record.nickname = nickname # Update nickname | ||
|
||
# Commit changes to the database | ||
db.session.commit() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not mandatory. check if you can use file_manager.modify_user you can maybe reuse the existing method
@@ -65,8 +65,11 @@ class User(db.Model): | |||
permissions = db.relationship('Permission', cascade='all,delete,delete-orphan', backref='user') | |||
authentication_backend = db.Column(db.String(255), nullable=False, default='local') | |||
|
|||
fullname = db.Column(db.String(255), nullable=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please show the failure
self.emailid.setPlaceholderText(_translate("addUserDialog", "[email protected]")) | ||
self.passwordLabel.setText(_translate("addUserDialog", "Password:")) | ||
self.password.setPlaceholderText(_translate("addUserDialog", "Your password")) | ||
self.confirmPasswordLabel.setText(_translate("addUserDialog", "Confirm Password:")) | ||
self.rePassword.setPlaceholderText(_translate("addUserDialog", "Confirm your password")) | ||
self.emailIDLabel.setText(_translate("addUserDialog", "Email:")) | ||
self.usernameLabel.setText(_translate("addUserDialog", "Username:")) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add only the fullname
@@ -247,3 +263,13 @@ def retranslateUi(self, MSColabConnectDialog): | |||
self.idpAuthTokenSubmitBtn.setText(_translate("MSColabConnectDialog", "Submit")) | |||
self.idpAuthTopicLabel.setText(_translate("MSColabConnectDialog", "Identity Provider Authentication")) | |||
self.statusLabel.setText(_translate("MSColabConnectDialog", "Status:")) | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the __name__
section gets in by some conversion option, we don't need that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okk
##Fixes #1282