Skip to content

Remove first_name / last_name from admin if missing from model - #512

Open
silviogutierrez wants to merge 3 commits into
jazzband:masterfrom
silviogutierrez:master
Open

Remove first_name / last_name from admin if missing from model#512
silviogutierrez wants to merge 3 commits into
jazzband:masterfrom
silviogutierrez:master

Conversation

@silviogutierrez

Copy link
Copy Markdown

Closes #509

This is a "static" approach, which I think is better than overriding get_search_fields. I believe the get_* methods are for truly dynamic behavior (based on the request, the current user, etc). With this, admin checks will still apply (though not particularly useful).

@codecov

codecov Bot commented Feb 13, 2023

Copy link
Copy Markdown

Codecov Report

Merging #512 (01ac6ec) into master (fcb6c4c) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #512      +/-   ##
==========================================
+ Coverage   94.22%   94.24%   +0.01%     
==========================================
  Files          30       30              
  Lines         900      903       +3     
==========================================
+ Hits          848      851       +3     
  Misses         52       52              
Impacted Files Coverage Δ
auditlog/admin.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@hramezani

Copy link
Copy Markdown
Member

@silviogutierrez Thanks for this path.

As I mention in the issue, overriding the get_search_fields would be a better approach. please implement it like this.
Also, you need to add some tests and a changelog entry in https://github.com/jazzband/django-auditlog/blob/master/CHANGELOG.md

Comment thread auditlog/admin.py
f"actor__{get_user_model().USERNAME_FIELD}",
]
f"actor__{user_model.USERNAME_FIELD}",
] + (

@RomHartmann RomHartmann Apr 4, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

How about something like the following, which adds whichever field is available, handling the case where either fist_name or last_name do actually exist

    search_fields = [
        "timestamp",
        "object_repr",
        "changes",
        f"actor__{get_user_model().USERNAME_FIELD}",
    ] + [f"actor__{f.name}" for f in get_user_model()._meta.get_fields() if f.name in ["first_name", "last_name"]]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is better because includes whatever fields that are available.
Feel free to create a new PR with tests and changelog

@twkrol

twkrol commented Jun 6, 2024

Copy link
Copy Markdown

Hi. Any progress planned for this issue?
It really prevents me from using search which results in big difficulty to get facts from the logs.

@hramezani

Copy link
Copy Markdown
Member

The approach of this PR should be changed.
Also test and change log has to be included.

BTW, you can create your custom admin inherited from LogEntryAdmin and implement your desired get_search_fields to unblock yourself or even better create a new PR and fix it for the library

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.

Document or remove first_name / last_name as a search in the admin for custom users

4 participants