doublesecretagency / craft-adwizard

Ad Wizard plugin for Craft CMS
Other
7 stars 8 forks source link

Add cache-proofing (to prevent Blitz conflict) #9

Open jsmrtn opened 4 years ago

jsmrtn commented 4 years ago

Related to #6 but I've not got an adblocker switched on. Running the latest version of adWizard, Craft 3.3.20.1.

We are using Blitz Cache (which might be relevant, as you mentioned this on the previous issue). It's injected onto the page using {{ craft.blitz.getTemplate('ism/includes/_adwizard') }}, and the include is pretty much some divs and the init for adWizard i.e. {{ craft.adWizard.randomizeAdGroup('leaderboard') }}

It's like it's not registering the asset bundles correctly on the front end, as I can't see the superagent.js and adwizard.js in my network tab.

Thanks!

jsmrtn commented 4 years ago

Morning Lindsey,

I think this is actually an issue with Blitz cache, as when I remove their proprietary code the assets load in correctly.

Kind regards,

Josh

jsmrtn commented 4 years ago

Probably is an adWizard issue in reality–see putyourlightson/craft-blitz#176. Not an issue in that it doesn't work, more it doesn't interface with Blitz correctly.

FWIW, I've put the code on the front end and hooked it up manually for now, so if this is a bit more of a long job then it's not hugely critical.

lindseydiloreto commented 4 years ago

Copy that, glad you found a good workaround!

@bencroker is right, this would be resolved by adding some of the same cache-proofing features that were just added to Upvote. Unfortunately that's a bit lower on my radar, so I can't say for certain when I'll get to it.

In the meantime, I'll leave this ticket open until I get around to cache-proofing Ad Wizard!

tomkiss commented 4 years ago

@joshua-martin Hello! Just wondering how you specifically fixed this?

Are you just literally manually loading in the JS files?

<script src="/cpresources/b776ac49/js/superagent.js?v=1599494050"></script>
<script src="/cpresources/b776ac49/js/aw.js?v=1599494050"></script>

Or something like that? That seems kind of wrong - I couldn't see how to access the js files directly from AdWizard in the docs.

jsmrtn commented 4 years ago

@tomkiss Even more manual than that–I copied superagent.js and aw.js verbatim from the resources folder and pasted it into our own custom scripts.

tomkiss commented 4 years ago

Ahhh, OK thanks.

For a moment I thought it could be called via registerJsFile - but this seems to output a root location path.

{% do view.registerJsFile('@doublesecretagency/adwizard/resources/superagent.js') %}

Thanks, will go the copy file route.

pgrzyb commented 3 years ago

Hey folks, I ran into the same issue a couple of days ago, and manually loading the JS files works fine-ish. The problem I'm facing right now is that AdWizard stopped counting ad clicks. AdWizard triggers a POST request on click to count it and it gets a 400 response with the Craft message "Bad Request - Unable to verify your data submission"

image

Any thoughts on this @lindseydiloreto? Is it somehow related to the CSRF token being wrong/missing due to blitz caching?

Thanks!

// EDIT:

Okay, I found a workaround. In the process, I understood that this issue is less related to Blitz than it is to AdWizard.

Anyway, I edited the manually copied JS files from AdWizard, I modified the adwizard.js file adding an extra fetch to get the token from the blitz action. I'm pasting it here in case someone else runs into the same problem:

// Ad Wizard JS
var adWizard = {
  click: function (id, url) {

      // Open link in new window
      window.open(url);

      fetch('/actions/blitz/csrf/token')
      .then(result => { 
          return result.text(); 
      })
      .then(result => { 
            // Set data
            var data = {'id':id};          

            // Add the csrfToken to the data object
            data[window.csrfTokenName] = result; 

            // Tally click
            window.superagent
                .post('/actions/ad-wizard/tracking/click')
                .send(data)
                .type('form')
                .set('X-Requested-With','XMLHttpRequest')
                .end(function (response) {
                    var message = JSON.parse(response.text);
                    console.log(message);
                })
            ;          
      })
  }
};

One more thing to note, you still need to add the csrf token name to the window object, for example by adding the script from the Craft docs to your layout file:

<script type="text/javascript">
    window.csrfTokenName = "{{ craft.app.config.general.csrfTokenName|e('js') }}";
 </script>

The token itself is useless here, since it would get cached by blitz anyway.

lindseydiloreto commented 3 years ago

Glad you found a workaround @pgrzyb. Thanks for the notes, I'll refer back to these when I get around to making Ad Wizard fully compatible with Blitz. 👍

mattsbanner commented 3 years ago

@lindseydiloreto Hello! 👋🏼

Do you have a target resolution time for this? We have quite a few sites live with Blitz and we're increasingly implementing AdWizard along side it, this is a bit of a blocker for us on some projects at the moment.

lindseydiloreto commented 2 years ago

Hi @mattsbanner, sorry for the late response! 👋

As you can tell by my response time, Ad Wizard stuff is currently a bit lower than a few other things on my plate. I'm not sure when it will receive another round of updates, though this ticket would be a high priority among them.

Did you try the workaround described above? Did that solution work out for you?

If you'd like to chat about speeding up the process, and/or would like to sponsor this new feature, I'd be happy to discuss it in greater detail. Feel free to ping me on Discord, perhaps something could be arranged. 👍