Skip to content

Commit cae763c

Browse files
committed
Fix sporadic MediaOrderConflictWarning issues in Django 2.1
fixes #141
1 parent 722d2ca commit cae763c

File tree

7 files changed

+264
-62
lines changed

7 files changed

+264
-62
lines changed

CHANGELOG.rst

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
11
Changelog
22
=========
33

4+
**3.3.2 (unreleased)**
5+
6+
* Fixed: Resolved sporadic MediaOrderConflictWarning issues on Django 2.2
7+
Fixes `#141`_.
8+
* Fixed: Improved media dependency ordering of widgets and nested_admin.js
9+
10+
.. _#141: https://github.com/theatlantic/django-nested-admin/issues/141
11+
412
**3.3.1 (Jun 1, 2020)**
513

614
* Fixed: ``show_change_link`` support in django-grappelli. Fixes `#173`_,

nested_admin/compat.py

Lines changed: 141 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44
See https://github.com/django/django/commit/c19b56f633e172b3c02094cbe12d28865ee57772
55
and https://code.djangoproject.com/ticket/28377
66
"""
7+
from collections import defaultdict, OrderedDict
78
import warnings
89

910
import django
1011
import django.forms
12+
from django.utils.datastructures import OrderedSet as _OrderedSet
1113

1214
try:
1315
from django.forms.widgets import MediaOrderConflictWarning
@@ -19,8 +21,57 @@ class MediaOrderConflictWarning(RuntimeWarning):
1921
else:
2022
MergeSafeMedia = django.forms.Media
2123

24+
try:
25+
from django.utils.topological_sort import (
26+
CyclicDependencyError, stable_topological_sort)
27+
except ImportError:
28+
class CyclicDependencyError(ValueError):
29+
pass
30+
31+
stable_topological_sort = None
32+
33+
34+
class OrderedSet(_OrderedSet):
35+
def __sub__(self, other):
36+
return self.__class__([i for i in self if i not in other])
37+
38+
def __str__(self):
39+
return "OrderedSet(%r)" % list(self)
40+
41+
def __repr__(self):
42+
return "OrderedSet(%r)" % list(self)
43+
44+
45+
if MergeSafeMedia is None or stable_topological_sort is None:
46+
def linearize_as_needed(l, dependency_graph):
47+
# Algorithm: DFS Topological sort
48+
# https://en.wikipedia.org/wiki/Topological_sorting#Depth-first_search
49+
l = list(dependency_graph)
50+
51+
temporary = set()
52+
permanent = set()
53+
54+
result = []
55+
56+
def visit(vertices):
57+
for vertex in vertices:
58+
if vertex in permanent:
59+
pass
60+
elif vertex in temporary:
61+
raise CyclicDependencyError('Cyclic dependency in graph')
62+
else:
63+
temporary.add(vertex)
64+
65+
visit(dependency_graph[vertex])
66+
67+
result.append(vertex)
68+
69+
temporary.remove(vertex)
70+
permanent.add(vertex)
71+
72+
visit(l)
73+
return result
2274

23-
if MergeSafeMedia is None:
2475
class MergeSafeMedia(django.forms.Media):
2576
def __init__(self, media=None, css=None, js=None):
2677
if media is not None:
@@ -31,59 +82,112 @@ def __init__(self, media=None, css=None, js=None):
3182
css = {}
3283
if js is None:
3384
js = []
34-
self._css = css
35-
self._js = js
85+
self._css_lists = [css]
86+
self._js_lists = [js]
87+
88+
@property
89+
def _css(self):
90+
css = defaultdict(list)
91+
for css_list in self._css_lists:
92+
for medium, sublist in css_list.items():
93+
css[medium].append(sublist)
94+
return {medium: self.merge(*lists) for medium, lists in css.items()}
95+
96+
@_css.setter
97+
def _css(self, data):
98+
css_media = ensure_merge_safe_media(django.forms.Media(css=data))
99+
self._css_lists = css_media._css_lists
100+
101+
@property
102+
def _js(self):
103+
return self.merge(*self._js_lists)
104+
105+
@_js.setter
106+
def _js(self, data):
107+
js_media = ensure_merge_safe_media(django.forms.Media(js=data))
108+
self._js_lists = js_media._js_lists
36109

37110
@staticmethod
38-
def merge(list_1, list_2):
111+
def merge(*lists):
39112
"""
40-
Merge two lists while trying to keep the relative order of the elements.
41-
Warn if the lists have the same two elements in a different relative
42-
order.
113+
Merge lists while trying to keep the relative order of the elements.
114+
Warn if the lists have the same elements in a different relative order.
43115
44116
For static assets it can be important to have them included in the DOM
45117
in a certain order. In JavaScript you may not be able to reference a
46118
global or in CSS you might want to override a style.
47119
"""
48-
# Start with a copy of list_1.
49-
combined_list = list(list_1)
50-
last_insert_index = len(list_1)
51-
# Walk list_2 in reverse, inserting each element into combined_list if
52-
# it doesn't already exist.
53-
for path in reversed(list_2):
54-
try:
55-
# Does path already exist in the list?
56-
index = combined_list.index(path)
57-
except ValueError:
58-
# Add path to combined_list since it doesn't exist.
59-
combined_list.insert(last_insert_index, path)
60-
else:
61-
if index > last_insert_index:
62-
warnings.warn(
63-
'Detected duplicate Media files in an opposite order:\n'
64-
'%s\n%s' % (combined_list[last_insert_index], combined_list[index]),
65-
MediaOrderConflictWarning,
66-
)
67-
# path already exists in the list. Update last_insert_index so
68-
# that the following elements are inserted in front of this one.
69-
last_insert_index = index
70-
return combined_list
120+
dependency_graph = OrderedDict()
121+
all_items = OrderedSet()
122+
123+
for list_ in filter(None, lists):
124+
head = list_[0]
125+
# The first items depend on nothing but have to be part of the
126+
# dependency graph to be included in the result.
127+
dependency_graph.setdefault(head, OrderedSet())
128+
for item in list_:
129+
all_items.add(item)
130+
# No self dependencies
131+
if head != item:
132+
dependency_graph.setdefault(item, OrderedSet())
133+
dependency_graph[item].add(head)
134+
head = item
135+
try:
136+
return linearize_as_needed(all_items, dependency_graph)
137+
except CyclicDependencyError:
138+
warnings.warn(
139+
'Detected duplicate Media files in an opposite order: {}'.format(
140+
', '.join(repr(l) for l in lists)
141+
), MediaOrderConflictWarning,
142+
)
143+
return list(all_items)
71144

72145
def __add__(self, other):
73146
combined = MergeSafeMedia()
74-
combined._js = self.merge(self._js, other._js)
75-
combined._css = {
76-
medium: self.merge(self._css.get(medium, []), other._css.get(medium, []))
77-
for medium in set(self._css.keys()) | set(other._css.keys())
78-
}
147+
other_css_lists = getattr(other, '_css_lists', None)
148+
if other_css_lists is None:
149+
other_css_lists = []
150+
for medium, css_list in other._css.items():
151+
for css_file in css_list:
152+
other_css_lists.append({medium: [css_file]})
153+
other_js_lists = getattr(other, '_js_lists', None) or [[js] for js in other._js]
154+
combined._css_lists = self._css_lists + other_css_lists
155+
combined._js_lists = self._js_lists + other_js_lists
79156
return combined
80157

81158
def add_js(self, data):
82159
if data:
83160
new_media = self + MergeSafeMedia(js=data)
84-
self._js = new_media._js
161+
self._js_lists = new_media._js_lists
85162

86163
def add_css(self, data):
87164
if data:
88165
new_media = self + MergeSafeMedia(css=data)
89-
self._css = new_media._css
166+
self._css_lists = new_media._css_lists
167+
168+
169+
def ensure_merge_safe_media(media):
170+
if isinstance(media, MergeSafeMedia):
171+
return media
172+
safe_media = MergeSafeMedia()
173+
for js in media._js:
174+
safe_media += MergeSafeMedia(js=[js])
175+
for medium, css_files in media._css.items():
176+
for css_file in css_files:
177+
safe_media += MergeSafeMedia(css={medium: [css_file]})
178+
return safe_media
179+
180+
181+
try:
182+
import polymorphic.admin.inlines
183+
import polymorphic.formsets.utils
184+
import polymorphic.formsets.models
185+
except ImportError:
186+
pass
187+
else:
188+
def add_media(dest, media):
189+
return dest + media
190+
191+
polymorphic.formsets.utils.add_media = add_media
192+
polymorphic.formsets.utils.add_media = add_media
193+
polymorphic.formsets.models.add_media = add_media

nested_admin/formsets.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
import six
1212
from six.moves import range
1313

14+
from .compat import ensure_merge_safe_media
15+
1416
if six.PY2:
1517
from django.utils.encoding import force_text as force_str
1618
else:
@@ -69,6 +71,11 @@ def __init__(self, *args, **kwargs):
6971
'parent_formset': self,
7072
})
7173

74+
@property
75+
def media(self):
76+
media = super(NestedInlineFormSetMixin, self).media
77+
return ensure_merge_safe_media(media)
78+
7279
def _construct_form(self, i, **kwargs):
7380
defaults = {}
7481
if '-empty-' in self.prefix:

nested_admin/nested.py

Lines changed: 74 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from django.utils.translation import gettext
1414
from django.contrib.admin.options import ModelAdmin, InlineModelAdmin
1515

16-
from .compat import MergeSafeMedia
16+
from .compat import ensure_merge_safe_media, MergeSafeMedia
1717
from .formsets import NestedInlineFormSet, NestedBaseGenericInlineFormSet
1818

1919

@@ -40,7 +40,40 @@ def get_model_id(model_cls):
4040
server_data_js_url = lazy_reverse('nesting_server_data')
4141

4242

43-
class NestedInlineAdminFormsetMixin(object):
43+
class NestedAdminMixin(object):
44+
@property
45+
def _djn_js_deps(self):
46+
"""
47+
Returns a set of js files that, if present, ought to precede
48+
nested_admin.js in the media load order
49+
"""
50+
extra = '' if settings.DEBUG else '.min'
51+
52+
return {
53+
'admin/js/core.js',
54+
'admin/js/vendor/jquery/jquery%s.js' % extra,
55+
'admin/js/jquery.init.js',
56+
'admin/js/prepopulate%s.js' % extra,
57+
'admin/js/SelectFilter2.js',
58+
'admin/js/autocomplete.js',
59+
'jquery.grp.autocomplete_fk.js',
60+
'grappelli/js/grappelli.js',
61+
'grappelli/js/grappelli.min.js',
62+
'grappelli/js/jquery.grp_autocomplete_fk.js',
63+
'grappelli/js/jquery.grp_autocomplete_generic.js',
64+
'grappelli/js/jquery.grp_autocomplete_m2m.js',
65+
'grappelli/js/jquery.grp_collapsible.js',
66+
'grappelli/js/jquery.grp_collapsible_group.js',
67+
'grappelli/js/jquery.grp_related_fk.js',
68+
'grappelli/js/jquery.grp_related_generic.js',
69+
'grappelli/js/jquery.grp_related_m2m.js',
70+
'grappelli/js/jquery.grp_timepicker.js',
71+
'grappelli/jquery/ui/jquery-ui.js',
72+
'grappelli/jquery/ui/jquery-ui.min.js',
73+
}
74+
75+
76+
class NestedInlineAdminFormsetMixin(NestedAdminMixin):
4477

4578
classes = None
4679

@@ -93,31 +126,27 @@ def __iter__(self):
93126

94127
@property
95128
def media(self):
96-
media = self.opts.media
97-
if not isinstance(media, MergeSafeMedia):
98-
media = MergeSafeMedia(media)
99-
media = media + self.formset.media
129+
media = ensure_merge_safe_media(self.opts.media)
130+
131+
media += ensure_merge_safe_media(self.formset.media)
132+
100133
for fs in self:
101-
media = media + fs.media
134+
media += ensure_merge_safe_media(fs.media)
102135
for inline in (getattr(fs.form, 'inlines', None) or []):
103-
media = media + inline.media
136+
media += ensure_merge_safe_media(inline.media)
104137

105138
min_ext = '' if getattr(settings, 'NESTED_ADMIN_DEBUG', False) else '.min'
139+
nested_admin_js_file = 'nested_admin/dist/nested_admin%s.js' % min_ext
106140

107-
js_file = 'nested_admin/dist/nested_admin%s.js' % min_ext
141+
media += MergeSafeMedia(js=[nested_admin_js_file])
108142

109-
enable_server_data_js = 'grappelli' in settings.INSTALLED_APPS
143+
# Check for override js files, and if present ensure that nested_admin.js
144+
# will follow them when ordered through a topological sort
145+
for js_file in media._js:
146+
if js_file in self._djn_js_deps:
147+
media += MergeSafeMedia(js=[js_file, nested_admin_js_file])
110148

111-
media += MergeSafeMedia(
112-
js=([server_data_js_url] if enable_server_data_js else []),
113-
css={'all': (
114-
'nested_admin/dist/nested_admin%s.css' % min_ext,
115-
)})
116-
117-
media_js = media._js
118-
if js_file not in media_js:
119-
media_js += [js_file]
120-
return MergeSafeMedia(js=media_js, css=media._css)
149+
return media
121150

122151
@property
123152
def inline_model_id(self):
@@ -211,10 +240,34 @@ class NestedInlineAdminFormset(NestedInlineAdminFormsetMixin, NestedBaseInlineAd
211240
pass
212241

213242

214-
class NestedModelAdminMixin(object):
243+
class NestedModelAdminMixin(NestedAdminMixin):
215244

216245
inline_admin_formset_helper_cls = NestedInlineAdminFormset
217246

247+
@property
248+
def media(self):
249+
media = ensure_merge_safe_media(super(NestedModelAdminMixin, self).media)
250+
251+
min_ext = '' if getattr(settings, 'NESTED_ADMIN_DEBUG', False) else '.min'
252+
nested_admin_js_file = 'nested_admin/dist/nested_admin%s.js' % min_ext
253+
254+
media_js = []
255+
if 'grappelli' in settings.INSTALLED_APPS:
256+
media_js.append(server_data_js_url)
257+
media_js.append(nested_admin_js_file)
258+
259+
media += MergeSafeMedia(
260+
js=media_js,
261+
css={'all': (
262+
'nested_admin/dist/nested_admin%s.css' % min_ext,
263+
)})
264+
265+
for js_file in media._js:
266+
if js_file in self._djn_js_deps:
267+
media += MergeSafeMedia(js=[js_file, nested_admin_js_file])
268+
269+
return media
270+
218271
def get_inline_formsets(self, request, formsets, inline_instances,
219272
obj=None, allow_nested=False):
220273
inline_admin_formsets = []

0 commit comments

Comments
 (0)