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

Jump to bottom

Add Last Login Date block to dashboard.html #4735

Merged
merged 13 commits into from Dec 3, 2020
Merged

Add Last Login Date block to dashboard.html #4735

merged 13 commits into from Dec 3, 2020

Conversation

vasantvohra
Copy link
Contributor

@vasantvohra vasantvohra commented Dec 3, 2020 •
edited

Need to add an empty block to the user dashboard template for displaying the last login date from the UN template override.

ThiefMaster
ThiefMaster requested changes Dec 3, 2020
View reviewed changes
Copy link
Member

@ThiefMaster ThiefMaster left a comment

Choose a reason for hiding this comment

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

Please don't keep your JIRA IDs in commit messages going upstream and use present tense :)

Add xxx block in dashboard template for example

indico/modules/users/templates/dashboard.html Outdated
@@ -49,6 +49,7 @@ <h3>{{ user.full_name }}</h3>
</div>
{% endif %}
{{ labels(user) }}
{% block last_login_dt %} {% endblock %}
Copy link
Member

Choose a reason for hiding this comment

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

The name is too specific. Please find a generic name. after_user_labels maybe. Also, no need to put a space in the default content.

@ThiefMaster
Copy link
Member

Showing the last login date may be useful in the core. Are you using the highest safe_last_login_dt of all the user's identities for this? If yes, maybe contributing the actual feature of showing the last login timestamp upstream would be better than just adding the block for you to override?

@vasantvohra vasantvohra changed the title IND2-1073 Added Last Login Date block Added Last Login Date block Dec 3, 2020
@vasantvohra
Copy link
Contributor Author

Showing the last login date may be useful in the core. Are you using the highest safe_last_login_dt of all the user's identities for this? If yes, maybe contributing the actual feature of showing the last login timestamp upstream would be better than just adding the block for you to override?

@ThiefMaster Yes, I'm using the user.local_identity.safe_last_login_dt with format_datetime function in the template, I guess the user.local_identity gives the most recently used local identity. Correct me, if I'm wrong. Also, In my UN Jira ticket, we are showing it only when {% if session.user.is_admin %} does the core wants this feature for all the users?
Apologize for the above silly mistakes just had an intro session from @OmeGak :)

@ThiefMaster
Copy link
Member

Since you can only view your own dashboard unless you are an admin, I think showing it all the time would be fine. I don't have objections in showing it only to admins there, since we eventually want to add a nice sessions overview for users as well where we could show it to the user themself.

@vasantvohra
Copy link
Contributor Author

since we eventually want to add a nice sessions overview for users as well where we could show it to the user themself.

Sessions overview will be good let me know the features you are thinking to add there. Also, I think we can show users last login location from the IP we are storing in identities. For the time being, I've committed the entire block, please have a review and let me know if any changes required.

ThiefMaster
ThiefMaster reviewed Dec 3, 2020
View reviewed changes
indico/modules/users/templates/dashboard.html Outdated Show resolved Hide resolved
indico/modules/users/templates/dashboard.html Outdated
@@ -49,6 +49,14 @@ <h3>{{ user.full_name }}</h3>
</div>
{% endif %}
{{ labels(user) }}
{% block session_details %}
Copy link
Member

Choose a reason for hiding this comment

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

Unless you still need to override it, let's get rid of the block around it.

indico/modules/users/templates/dashboard.html Outdated
Comment on lines 56 to 57
{% set last_login_dt = user.local_identity.safe_last_login_dt %}
<div class="detail">{{ last_login_dt | format_datetime }}</div>
Copy link
Member

Choose a reason for hiding this comment

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

This will fail if the user has no local_identity.

@vasantvohra vasantvohra changed the title Added Last Login Date block Add Last Login Date block to dashboard.html Dec 3, 2020
@vasantvohra vasantvohra closed this Dec 3, 2020
@ThiefMaster ThiefMaster reopened this Dec 3, 2020
@ThiefMaster ThiefMaster merged commit d390d76 into indico:2.3-maintenance Dec 3, 2020
@ThiefMaster ThiefMaster deleted the IND2-1073/last_login_dt branch December 3, 2020 19:30
vasantvohra added a commit to UNOG-Indico/indico-core that referenced this pull request Dec 7, 2020
@vasantvohra
Show last login date in dashboard (indico#4735)
f888abc
vasantvohra added a commit to UNOG-Indico/indico-core that referenced this pull request Dec 7, 2020
@vasantvohra
Show last login date in dashboard (indico#4735)
3ebfd7a
vasantvohra added a commit to UNOG-Indico/indico-core that referenced this pull request Dec 7, 2020
@vasantvohra
Show last login date in dashboard (indico#4735)
172d049
vasantvohra added a commit to UNOG-Indico/indico-core that referenced this pull request Dec 7, 2020
@vasantvohra
Show last login date in dashboard (indico#4735)
6526a3d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ThiefMaster ThiefMaster ThiefMaster requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@vasantvohra @ThiefMaster