-
Notifications
You must be signed in to change notification settings - Fork 3
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
3106 reparse django action #3167
base: develop
Are you sure you want to change the base?
Changes from 50 commits
2753507
f912d6b
b79cbb1
11b411e
1ca33a6
e5b9371
4a3a169
f810633
bc050cf
16bfbce
3fa6d7a
f91fc46
6c0cc70
3f7690c
fb6ecb7
198b98a
c61b298
b3504b9
e87808c
16ecfdb
d59af03
e1a3bbc
d3fd407
2237686
778f706
a5737b4
8e94085
7fa47a8
dd6d85e
1463ea4
152d439
d3deddf
f78e5ae
a54f14a
2b75af8
626087f
38d50f5
5e6c89c
ed965ac
95b6ebd
1c19e87
98bd857
e09ce5d
e100f6d
3daebc5
4f28656
f1c22f4
6ca4da4
f3a3959
4945d32
d1fb4d6
6e723e0
bf2d288
a1c4631
4d6dcb4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,16 @@ | ||
"""Admin class for DataFile objects.""" | ||
from django.contrib import admin | ||
from tdpservice.core.utils import ReadOnlyAdminMixin | ||
# from tdpservice.core.filters import custom_filter_title | ||
from tdpservice.data_files.models import DataFile, LegacyFileTransfer | ||
from tdpservice.parsers.models import DataFileSummary, ParserError | ||
from tdpservice.data_files.admin.filters import DataFileSummaryPrgTypeFilter, LatestReparseEvent, VersionFilter | ||
from django.conf import settings | ||
from django.utils.html import format_html | ||
from datetime import datetime, timedelta, timezone | ||
from django.shortcuts import redirect | ||
from django.core.management import call_command | ||
from django.utils.translation import ngettext | ||
from django.contrib import messages | ||
|
||
DOMAIN = settings.FRONTEND_BASE_URL | ||
|
||
|
@@ -23,11 +26,42 @@ def has_change_permission(self, request, obj=None): | |
"""Read only permissions.""" | ||
return False | ||
|
||
|
||
@admin.register(DataFile) | ||
class DataFileAdmin(ReadOnlyAdminMixin, admin.ModelAdmin): | ||
"""Admin class for DataFile models.""" | ||
|
||
class Media: | ||
"""Media class for DataFileAdmin.""" | ||
|
||
js = ('admin/js/admin/admin_datafile_model.js',) | ||
|
||
actions = ['reparse'] | ||
|
||
def reparse(self, request, queryset): | ||
"""Reparse the selected data files.""" | ||
files = queryset.values_list("id", flat=True) | ||
file_ids = ",".join(map(str, files)) | ||
call_command("clean_and_reparse", f"-f {file_ids}") | ||
self.message_user( | ||
request, | ||
ngettext( | ||
"%d file successfully submitted for reparsing.", | ||
"%d files successfully submitted for reparsing.", | ||
files.count(), | ||
) | ||
% files.count(), | ||
messages.SUCCESS, | ||
) | ||
return redirect("/admin/search_indexes/reparsemeta/") | ||
|
||
def get_actions(self, request): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: if not admin, then the reparse command is not shown |
||
"""Return the actions.""" | ||
actions = super().get_actions(request) | ||
if not request.user.groups.filter(name__in=["OFA System Admin", "OFA Admin"]).exists(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. The problem is then it will do two queries instead of one |
||
if "reparse" in actions: | ||
del actions["reparse"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of another There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately no since actions is a list and .pop only works with int indexes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you are right, it is a dictionary and although my original test failed, I tested again and it passed. so will do the change |
||
return actions | ||
|
||
def status(self, obj): | ||
"""Return the status of the data file summary.""" | ||
return DataFileSummary.objects.get(datafile=obj).status | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
$(window).on('load', function() { | ||
//your code here | ||
console.log('loaded'); | ||
var submitBtn=document.querySelector('button[type=submit]'); // add the first listener | ||
var theForm = submitBtn.parentNode.parentNode; | ||
|
||
for (var i = 0; i < theForm.childNodes.length; i++) { | ||
console.log(theForm.childNodes[i].className) | ||
if (theForm.childNodes[i].className == "actions") { | ||
form_header = theForm.childNodes[i]; | ||
break; | ||
} | ||
} | ||
for (var i = 0; i < form_header.childNodes.length; i++) { | ||
console.log(form_header.childNodes[i].className) | ||
if (form_header.childNodes[i].className == "action-counter") { | ||
number_of_files = form_header.childNodes[i]; | ||
break; | ||
} | ||
} | ||
submitBtn.addEventListener('click', function(e) { | ||
e.preventDefault(); | ||
disableFields(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. should be fixed now |
||
if (confirm("You are about to re-parse " + number_of_files.innerHTML.split(/(\s+)/)[0] + " files. Are you sure you want to continue?")) { | ||
console.log('submitting'); | ||
theForm.submit(); | ||
} else { | ||
console.log('not submitting'); | ||
}; | ||
}); | ||
|
||
|
||
}); |
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 works, but my preference would be the re-implement the logic of the management command here
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.
one reason for this - i was testing the sequential reparse scenario and was confronted with this error
i couldn't find the actual exception in the logs (the parse was still active, and putting out tons of logs), but if we re-implemented the command logic, that error could be caught and displayed to the user
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.
You should be able to catch the exception/log message in the admin console LogEntries. It should make a new entry indicating it wont reparse when not sequential
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 also agree with Jan that as a first pass calling the command works, but it would be nice to share the logic between this and the command to allow for a little more fine grain control.
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 are we ok to repeat the steps?
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 don't think they need to have code duplicates. I bet we can factor out parametrized functions from the command and put them into a shared module for both the command and the action to import. That will be a good amount of work though which I think could make sense to move into a follow on ticket.
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 can make a util file and create util functions and share them and I agree this can be a new ticket
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.
Created a new issue #3205