Skip to content
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

Ss 643 make an option to edit account details #235

Draft
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

anondo1969
Copy link
Contributor

@anondo1969 anondo1969 commented Oct 7, 2024

Status

Draft

Description

SS 643

Types of changes

new feature

Checklist

If you're unsure about any of the items below, don't hesitate to ask. We're here to help!
This is simply a reminder of what we are going to look for before merging your code.

  • This pull request is against develop branch (not applicable for hotfixes)
  • I have included a link to the issue on GitHub or JIRA (if any)
  • I have included migration files (if there are changes to the model classes)
  • I have included, reviewed and executed tests (unit and end2end) to complement my changes
  • I have updated the related documentation (if necessary)
  • I have added a reviewer for this pull request
  • I have added myself as an author for this pull request
  • In the case I have modified settings.py, then I have also updated the studio-settings-configmap.yaml file in serve-charts

Further comments

Anything else you think we should know before merging your code!

@anondo1969 anondo1969 self-assigned this Oct 7, 2024
common/views.py Outdated

if user_form_details.is_valid() and profile_form_details.is_valid():

user_form_details.save()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will have to be wrapped as a transaction I think

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#Addressed it as follows:

if user_form_details.is_valid() and profile_form_details.is_valid():
            try:
                with transaction.atomic():
                    user_form_details.save()
                    profile_form_details.save()

                    logger.info(
                        "Updated First Name: " + str(self.get_user_profile_info(request).user.first_name), exc_info=True
                    `)`
                    logger.info(
                        "Updated Last Name: " + str(self.get_user_profile_info(request).user.last_name), exc_info=True
                    )
                    logger.info(
                        "Updated Department: " + str(self.get_user_profile_info(request).department), exc_info=True
                    )

            except Exception as e:
                return HttpResponse("Error updating records: " + str(e))

<h3 class="card-title mb-0">My user profile</h3>
</div>
<div class="col">
<button type="button" type="submit" aria-expanded="false" class="btn btn-profile"><a href="{% url 'common:edit-profile' %}">Edit</a></button>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<button type="button" type="submit" aria-expanded="false" class="btn btn-profile"><a href="{% url 'common:edit-profile' %}">Edit</a></button>
<button type="button" type="submit" aria-expanded="false" class="btn btn-profile">
<a href="{% url 'common:edit-profile' %}">Edit</a>
</button>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed it.


<div class="row">
<div class="col">
<h3 class="card-title mb-0">My user profile</h3>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<h3 class="card-title mb-0">My user profile</h3>
<h3 class="card-title mb-0">My user profile</h3>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed the indent

@@ -0,0 +1,118 @@
{% extends 'base.html' %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd put this file in templates/user/ to other profile related templates

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to suggest also, we change how this form looks like down the line to match the look of profile view.

But right now we should get this form to just work logic-wise

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to templates/user/

Edited the form to look like Profile form.

{{form.email}}
<div class="form-text">{{form.email.help_text}}</div>
{% if form.email.errors %}
<div id="validation_email" class="invalid-feedback pt-1">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like there is no need for this if it's not editable

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And same goes for other fields

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added errors except the 'unchanged' ones.

common/views.py Outdated
logger.info("Updated Last Name: "+str(self.get_user_profile_info(request).user.last_name), exc_info=True)
logger.info("Updated Department: "+str(self.get_user_profile_info(request).department), exc_info=True)

return render(request, "registration/edit_profile_done.html")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally think, that redirect should lead back to the userprofile view.

Do you think that this page is necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now redirect goes to profile view.

common/views.py Outdated
@@ -206,7 +208,9 @@ def post(self, request, *args, **kwargs):
# was invalid

#print (form.errors)
logger.error("Edit user error: " + str(user_form_details.errors), exc_info=True)
logger.error("Edit profile error: " + str(profile_form_details.errors), exc_info=True)
if user_form_details.is_valid()==False:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if user_form_details.is_valid()==False:
if not user_form_details.is_valid():

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rewrote it.

common/views.py Outdated
logger.error("Edit profile error: " + str(profile_form_details.errors), exc_info=True)
if user_form_details.is_valid()==False:
logger.error("Edit user error: " + str(user_form_details.errors), exc_info=True)
if profile_form_details.is_valid()==False:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if profile_form_details.is_valid()==False:
if not profile_form_details.is_valid():

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rewrote it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants