Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jsmrtn opened this issue Feb 14, 2020 · 10 comments
Open

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

jsmrtn opened this issue Feb 14, 2020 · 10 comments
Labels
feature request New feature or request

Comments

@jsmrtn
Copy link

jsmrtn commented Feb 14, 2020

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
Copy link
Author

jsmrtn commented Feb 24, 2020

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 jsmrtn closed this as completed Feb 24, 2020
@jsmrtn jsmrtn reopened this Feb 24, 2020
@jsmrtn
Copy link
Author

jsmrtn commented Feb 24, 2020

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
Copy link
Contributor

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!

@lindseydiloreto lindseydiloreto added the feature request New feature or request label Feb 25, 2020
@lindseydiloreto lindseydiloreto changed the title adWizard is not defined Add cache-proofing (to prevent Blitz conflict) Feb 25, 2020
@tomkiss
Copy link

tomkiss commented Sep 7, 2020

@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
Copy link
Author

jsmrtn commented Sep 7, 2020

@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
Copy link

tomkiss commented Sep 7, 2020

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
Copy link
Contributor

pgrzyb commented May 6, 2021

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
Copy link
Contributor

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
Copy link

@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
Copy link
Contributor

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. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants