1. OpenREM Avatar OpenREM
  2. OpenREM
  3. OpenREM
  4. Issues

reverse_js config error

Issue #758 resolved
Ed McDonagh created an issue 
(virtualenv) deploy@localhost:~/sites/demo.openrem.org/source/openrem$ python manage.py collectstatic_js_reverse
js-reverse file written to /home/deploy/sites/demo.openrem.org/static/django_js_reverse/js
(virtualenv)

But in nginx logs:

2019/05/17 21:38:12 [error] 30627#30627: *202881 open() "/home/deploy/sites/demo.openrem.org/static/js/django_reverse/reverse.js" failed (2: No such file or directory), client: 88.98.88.16, server: demo.openrem.org, request: "GET /static/js/django_reverse/reverse.js HTTP/1.1", host: "demo.openrem.org", referrer: "http://demo.openrem.org

Comments (26)

  1. Ed McDonagh reporter

    @LuukO Is there something obvious I've done wrong that would lead to the two paths being different?

  2. Ed McDonagh reporter
    • edited description 
    (virtualenv) deploy@localhost:~/sites/demo.openrem.org/source/openrem$ python manage.py collectstatic_js_reverse
js-reverse file written to /home/deploy/sites/demo.openrem.org/static/django_js_reverse/js(virtualenv)

    But in nginx logs:

    2019/05/17 21:38:12 [error] 30627#30627: *202881 open() "/home/deploy/sites/demo.openrem.org/static/js/django_reverse/reverse.js" failed (2: No such file or directory), client: 88.98.88.16, server: demo.openrem.org, request: "GET /static/js/django_reverse/reverse.js HTTP/1.1", host: "demo.openrem.org", referrer: "http://demo.openrem.org
  3. Ed McDonagh reporter

    @LuukO - on my production system the collectstatic_js_reverse command writes to /var/dose/static/django_js_reverse/js but in the webserver logs I can see that the file that is being used is at /static/js/django_reverse/reverse.js - which does exist…

    On the demo server, we have the writing and reading as per the original post to this issue. But in this case, it doesn't exist. But there is a folder ~/sites/demo.openrem.org/static/django_js_reverse/js - i.e. it has an extra _js_ in the middle.

    What’s going on, and how do we fix it!

  4. Ed McDonagh reporter

    @dplatten I’m finding that the event filtering works, but the charts never render (and the spinner keeps spinning). For Mean DLP per acquisition, I get the following console error when I have set the spiral filter to 2:

    chartUpdateData.js:227 Uncaught TypeError: Cannot read property 'num' of undefined
    at updateAverageChart (chartUpdateData.js:227)
    at Object.success (ctChartAjax.js:26)
    at j (jquery-1.11.0.min.js:2)
    at Object.fireWith [as resolveWith] (jquery-1.11.0.min.js:2)
    at x (jquery-1.11.0.min.js:4)
    at XMLHttpRequest.b (jquery-1.11.0.min.js:4)

        else {
        for (j = 0; j < nameList.length; j++) {
            currentCounts = 0;
            currentValue = 0;
            for (i = 0; i < systemList.length; i++) {
                if (summaryData[i][j].num === null) {summaryData[i][j].num = 0;}
                currentCounts += parseFloat(summaryData[i][j].num);
                if (averageChoice === "mean") {
                    if (summaryData[i][j].mean === null) {summaryData[i][j].mean = 0;}
                    currentValue += parseFloat(summaryData[i][j].num) * parseFloat(summaryData[i][j].mean);
                }
                else if (averageChoice === "median") {
                    if (summaryData[i][j].median === null) {summaryData[i][j].median = 0;}
                    currentValue += parseFloat(summaryData[i][j].num) * parseFloat(summaryData[i][j].median);
                }
                else {
                    if (summaryData[i][j].mean === null) {summaryData[i][j].mean = 0;}
                    if (summaryData[i][j].median === null) {summaryData[i][j].median = 0;}
                    currentValue += parseFloat(summaryData[i][j].num) * parseFloat(summaryData[i][j].mean);
                }
            }
            totalCountsPerName.push(currentCounts);
            averageValuePerName.push(currentValue / currentCounts);

    Line 227 is if (summaryData[i][j].num === null) {summaryData[i][j].num = 0;}

  5. David Platten

    @edmcdonagh that’s the problem that I’ve found too. The root of the problem is in the chart_functions.py file on line 150. This line appends the median values to the results. It works perfectly if none of the new filters are applied, but results in an empty queryset if the new filters are used. I don’t know what. I’ve stopped the code at this point and played around.

    ‌

    Line 150 is:

    return_structure['summary'].append(database_events.values('db_series_names_to_use').annotate(**summary_annotations).order_by('db_series_names_to_use'))

    ‌

    If you put a breakpoint at this line and run the code using the debugger python prompt without the “order_by” part then you see results, which are of the same format whether you have the new filters applied or not. If you add the “order_by” part to the code, then you get a blank result when using the new filters, but the correct values without. I don’t understand what’s going on.

  7. Ed McDonagh reporter

    Maybe not the recreation of the querysets because if I make the following change to _specify_event_numbers_spiral in mod_filters.py

        else:
        # study_uids = queryset.values_list('study_instance_uid')
        # filtered = GeneralStudyModuleAttr.objects.filter(study_instance_uid__in=study_uids).filter(
        filtered = queryset.filter(
            ctradiationdose__ctirradiationeventdata__ct_acquisition_type__code_meaning__exact='Spiral Acquisition'
        ).annotate(
            spiral_count=Count('ctradiationdose__ctirradiationeventdata', distinct=True)
        ).filter(spiral_count=value)
    return filtered

    it still hangs in the same way when I use the spiral event number filter.

  9. Ed McDonagh reporter

    Thank you @LuukO . At the moment, I think they are unrelated issues, but we need to fix both of them!

  10. David Platten

    I wonder whether this is an “opportunity” to restructure the database tables to a flatter structure? a study-based table could include # events, # spiral events etc so these could be more easily accessed for charting purposes.

  11. Ed McDonagh reporter

    I’m not sure there are many tables we can flatten (I could well be wrong), as they are based on the templates which mostly allow multiples.

    But I've often thought about having a summary table that duplicates key information to make the queries easier - totals and number of events etc would certainly be candidates.

    And we could write a database migration to populate it on upgrade for the existing studies.

  12. Luuk

    Concerning the django-reverse issue:

    The (at least current, maybe also former) implementation of django-js-reverse puts the reverse.js in os.path.join(settings.STATIC_ROOT, 'django_js_reverse', 'js'). In Openrem we use <script src="{{ STATIC_URL }}js/django_reverse/reverse.js"></script>. So that doesn’t match.

    I see three solutions:

    1. Change the location in 6 html-templates (rffiltered.html, mgfiltered.html, rfdetail.html, ctfiltered.html, dxfiltered.html, dicomqr.html)
    2. Change the location that manage.py collectstatic_js_reverse writes the output (by defining JS_REVERSE_OUTPUT_PATH in settings.py)
    3. Let the user copy the file manually

    I think option 3 is not user-friendly and option 1 is possible, but then the reverse.js file is not located in the same directory as all the other javascript files (under the js directory). So I would suggest to define JS_REVERSE_OUTPUT_PATH in settings.py as JS_REVERSE_OUTPUT_PATH = os.path.join(STATIC_ROOT, 'js', 'django_reverse'). It is probably also the most future-proof solution as you fix the output directory and are not dependent on location changes in future releases of django-js-reverse (if this happens).

    @edmcdonagh , @dplatten , what do you think?

  13. Ed McDonagh reporter

    I’m happy with 2. What effect is the current mismatch likely to have on non-virtual directory installs @LuukO ?

  14. Ed McDonagh reporter

    If you define JS_REVERSE_OUTPUT_PATH in settings.py does it matter that STATIC_ROOT gets (re)defined in local_settings.pyafterwards?

  15. Luuk

    On current installs (and new installs) using non-virtual directory there will be no effect. As the supplied reverse.js is in the directory that matches the directory in the html-templates.

    I edited below as I thought Ed was “happy with option 1” (strange behaviour of my brain).

    If we use option 2 you should indeed overwrite also the JS_REVERSE_OUTPUT_PATH if you change STATIC_ROOT. Although it can be the same line: JS_REVERSE_OUTPUT_PATH = os.path.join(STATIC_ROOT, 'js', 'django_reverse').

    Option 1 would prevent that step for everyone changing STATIC_ROOT (also running in non-virtual directory), that would be a pro for option 1. Although we could add the line by default in the example local_settings.py.

  16. Luuk

    It is not completely correct what I thought yesterday evening. As we deliver the correct “reverse.js” you don’t need JS_REVERSE_OUTPUT_PATH unless you need to create a new one (only in case of a web site in a virtual-directory). So what we need to do:

    1. Define JS_REVERSE_OUTPUT_PATH in settings.py
    2. Add JS_REVERSE_OUTPUT_PATH also in local_settings.py.example (just to make sure that if you change STATIC_ROOT, JS_REVERSE_OUTPUT_PATH is updated accordingly)
    3. Update the upgrade documentation to instruct users to add it to the local_settings.py file in case you are running a virtual environment (or to be on the safe side if you changed STATIC_ROOT).

    @edmcdonagh agree?

  18. Luuk

    Refs issue #758 reverse.js in correct location

    • Updated local_settings.py.example to get reverse.js in the correct locatin if "manage.py collectstatic_js_reverse" is applied
    • Updated documentation of 0.9.1 release to instruct users to update local_settings.py accordingly (only in case of running website in Virtual directory as otherwise it has no effect)

    Update of settings.py is not necessary as STATIC_ROOT is not defined here (anymore)

    → <<cset 7281ce9ddef0>>

  25. Ed McDonagh reporter

    Was: Possible event filter/chart/reverse_js clash

    Event filter and chart clash has effectively been dealt with in ref #759. There is discussion below, but this has been superseded by the changes in #759.

  27. Log in to comment
Assignee
–
Type
bug
Priority
major
Status
resolved
Component
–
Milestone
0.10.0
Votes
0
Watchers
1
Jira: the preferred issue tracker for Bitbucket. Join the team!