search

ampproject / amphtml

The AMP web component framework.
https://amp.dev
Apache License 2.0
14.89k stars 3.89k forks source link

✨ Add new analytics variables for Navegg vendor #40194

Closed hugocs1 closed 3 weeks ago

hugocs1 commented 3 weeks ago
CLAassistant commented 3 weeks ago

CLA assistant check
All committers have signed the CLA.

erwinmombay commented 3 weeks ago

LGTM

erwinmombay commented 3 weeks ago

@hugocs1 could you get the failing tests to pass. will merge when green:

TOTAL: 1 FAILED, 2209 SUCCESS

1) test requests
     amp-analytics   vendor request tests analytics vendor: navegg
     Error: Vendor navegg, request pageview doesn't match. Expected value https://amp.navdmp.com/amp?aid=_client_id(navegg_id)_&url=_canonical_url_&ref=_document_referrer_&tit=_title_&lan=_browser_language_&acc=!account&wst=_wst_&wct=_wct_&wla=_wla_&v=7, last sent out value is https://amp.navdmp.com/amp?aid=_client_id(navegg_id)_&url=_canonical_url_&ref=_document_referrer_&tit=_title_&lan=_browser_language_&acc=!account&wst=!wst&wct=!wct&wla=!wla&v=7.
hugocs1 commented 3 weeks ago

@erwinmombay done!

powerivq commented 3 weeks ago

@hugocs1 Suggest rebasing against the latest main branch to have the CI build fixed.

hugocs1 commented 3 weeks ago

@powerivq you mean this one? I tried but it says "This branch is not behind the upstream".

Maybe re-run the CI?

Thanks in advance!

powerivq commented 3 weeks ago

@hugocs1 Sorry, please rebase now! It should have the fix.