search

OpenTechStrategies / streetcrm

StreetCRM is a free software contact management application
Other
5 stars 4 forks source link

Upgrade all dependencies. #345

Open kfogel opened 5 years ago

kfogel commented 5 years ago

It's been a while. Let's do this. See also issue #326.

kfogel commented 5 years ago

Resolving this would include resolving any security issues, such as those detected by GitHub as per this screenshot:

Screenshot from 2019-03-12 15-00-15

@webmaven, if you're logged in to GitHub, you see that same warning as in the screenshot when you visit this repository, right? If not, please let me know and I'll adjust your access rights as needed.

webmaven commented 5 years ago

@kfogel, I don't see that warning.

kfogel commented 5 years ago

Gotcha, @webmaven. Okay, I've just invited you to the StreetCRM-Admin team. After you accept that invitation, re-log-in and see if the notice shows up for you when visiting this repository.

webmaven commented 5 years ago

My apologies for the delay. Thanks, I see the warning now.

kfogel commented 5 years ago

Great, thanks @webmaven. By the way, @frankduncan is working on some of the other high-priority labeled issues right now, and the issue assignments reflect that. I've got this one on your plate because you have the longest/broadest Python experience of any of us (whether Django specifically, I don't know, but at any rate you're probably going to be better at debugging the kinds of problems that come up in a deps upgrade like this).

Once we get the Django upgrade and deps issues solved, there are other functional improvements to make, but... you know: security first and all that.

frankduncan commented 5 years ago

@webmaven (cc @kfogel ). I did some work upgrading django as a first pass before giving up where forms.py was calling into ModelChoiceField (line 172) with a string and that appeared to want a queryset. I put my stuff in https://github.com/OpenTechStrategies/streetcrm/tree/upgrade-django with each change broken out into its own commit. Let me know if you have any questions about how I came to the conclusions I did.

webmaven commented 5 years ago

Okay, seems reasonable. BTW, @kfogel, I've self-assigned the other bug and technical debt issues, using similar logic to yours.

@frankduncan, nice! Okay, will query as needed, thanks.

frankduncan commented 5 years ago

Did a bit more work on this, and got stymied, so dumping my notes here for posterity:

So, there's two major pain points on the upgrade which are beginning to spiral out of control. The first is that streetcrm makes heavy use of django-admin-bootstrapped, up to and including copying some code out of there into streetcrm for modification purposes (see https://github.com/OpenTechStrategies/streetcrm/blob/master/streetcrm/templates/admin/change_list.html). For background, django-admin-bootstrapped was a django addon to make the admin interface use the twitter bootstrap stuff which was all the rage a few years ago. The big problem is that django-admin-bootstrapped only works with django 1.8, and I've run into a few places where this actually breaks at runtime. It looks like we're faced with a choice: detangle DAB from streetcrm, or fork DAB.

The second pain point is that the move from django 1.8 to django 1.11 (and then to 2.x) involves a substantial difference in how django admin model stuff is overridden. I've run into a few places so far, but one of the challenges is due to these being runtime issues, the only way to see if something is different is comparing against the previous version when running. The one I was looking at today was that custom widgets and their templates are defined differently, which was non obvious because the old method, override render(), is replaced by templates and delcarations. This affected how the autocomplete stuff was getting swapped in for selects.

That second pain point is compounded by the fact that the autocompletion library that swoptact uses has changed to be in line with the django changes.