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

Fixes #1501 - Add username to the export #1538

Merged
merged 2 commits into from
Sep 13, 2023

Conversation

jonespm
Copy link
Member

@jonespm jonespm commented Sep 12, 2023

No description provided.

@jonespm jonespm linked an issue Sep 12, 2023 that may be closed by this pull request
dashboard/admin.py Outdated Show resolved Hide resolved
@@ -115,8 +116,14 @@ def save_model(self, request, obj, form, change):
return super(CourseAdmin, self).save_model(request, obj, form, change)

class LogResource(resources.ModelResource):

# This field is used to get the username from the user model
username = Field(attribute='user__username', column_name='username')
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional: is the column_name here giving a custom name to the data field as part of the auth_user.username. If yes, will it make sense to name it as user_uniq_id?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes that's the header in the CSV, otherwise it was user__username. Can change it to something else. This just matched up with the column in the other table.

Copy link
Member Author

@jonespm jonespm Sep 13, 2023

Choose a reason for hiding this comment

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

I'm not sure, it does represent the username column in the auth_user table. We could call it auth_user__username maybe or left it as user__username. I think for purposes of the CSV as a column header it's fine as the shorter username or the other way. I don't think any admin using this is going to care either way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah.. I agree

Copy link
Contributor

@pushyamig pushyamig left a comment

Choose a reason for hiding this comment

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

Some minor suggestion on code comments and username change. The change looks good and works as expected

@jonespm jonespm merged commit 9018413 into tl-its-umich-edu:master Sep 13, 2023
@jonespm jonespm deleted the issue_1501 branch September 13, 2023 14:55
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.

Add the username to the csv output in the MyLA event log data
3 participants