savoirfairelinux / sous-chef

Sous-Chef is a web application to help organizations to plan and deliver meals, and to manage clients files.
GNU Affero General Public License v3.0
67 stars 45 forks source link

Remove the use of CDN for include the CSS and JavaScript in the page new orders, calendar date selection. #790

Open erozqba opened 7 years ago

erozqba commented 7 years ago

Expected Behaviour

We only use CSS and JavaScript inlined from /static/

Actual Behaviour

In the page new orders for the calendar date selection, we are using externally hosted libraries.

Steps to reproduce

kousu commented 6 years ago

Hi! I think this duplicates #767 , which I think is great because it's nice to see these polish corners being taken care of.

But even though it duplicates, it's possible there's still an issue.

First, the version installed on sous-chef-test right now is, apparently, https://github.com/savoirfairelinux/sous-chef/commit/396f86d696fd1dc541b6556dacea8a621e2b7420, on January 10th, which is after #767 was fixed. But if I go to https://sous-chef-test.santropolroulant.org/login right now and look at the source i see

    <link href="https://cdnjs.cloudflare.com/ajax/libs/semantic-ui/2.1.8/semantic.min.css" rel="stylesheet">
...
    <script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/2.2.2/jquery.min.js" type="application/javascript"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/semantic-ui/2.1.8/semantic.min.js" type="application/javascript"></script>

also, I notice that those version numbers are mismatch the version numbers in https://github.com/savoirfairelinux/sous-chef/pull/779/files (2.2.2 vs 2.1.4 and 2.1.8 vs 2.2.10). What's going on? Was there a bad merge somewhere?

I checked the git log on sous-chef-test and src/sous_chef/templates/base.html shows that that commit (i.e. #779) was indeed applied.

Rather, what's happened is that there are other templates which still include files from cloudflare:

root@sous-chef-test:/home/sfl/sous-chef# git grep cdnjs
src/delivery/templates/edit_delivery_of_today.html:        <script src="https://cdnjs.cloudflare.com/ajax/libs/mustache.js/2.3.0/mustache.js"></script>
src/delivery/templates/edit_delivery_of_today.html:        <script src="https://cdnjs.cloudflare.com/ajax/libs/mustache.js/2.3.0/mustache.min.js"></script>
src/order/templates/order/create_batch.html:<link href="https://cdnjs.cloudflare.com/ajax/libs/semantic-ui/2.2.11/semantic.min.css" rel="stylesheet">
src/order/templates/order/create_batch.html:    <script src="https://cdnjs.cloudflare.com/ajax/libs/jqueryui/1.11.1/jquery-ui.min.js" type="application/javascript"></script>
src/order/templates/order/create_batch.html:    <script src="https://cdnjs.cloudflare.com/ajax/libs/jqueryui/1.11.1/i18n/jquery-ui-i18n.min.js" type="application/javascript"></script>
src/order/templates/order/create_batch.html:    <script src="https://cdnjs.cloudflare.com/ajax/libs/semantic-ui/2.2.11/semantic.min.js" type="application/javascript"></script>
src/sous_chef/templates/splash.html:    <link href="https://cdnjs.cloudflare.com/ajax/libs/semantic-ui/2.1.8/semantic.min.css" rel="stylesheet">
src/sous_chef/templates/splash.html:    <script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/2.2.2/jquery.min.js" type="application/javascript"></script>
src/sous_chef/templates/splash.html:    <script src="https://cdnjs.cloudflare.com/ajax/libs/semantic-ui/2.1.8/semantic.min.js" type="application/javascript"></script>

So I would say that #779 was a valiant attempt and thanks very much to @lingxiaoyang for writing it, but it wasn't quite enough to finish #767. Shall we just call #790 the sequel and continue working here?

erozqba commented 6 years ago

@kousu Yes! Thanks for providing all this information @kousu I will try to continue this work and remove the use of CDN.