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

Rtd extend bug fix #5651

Closed
wants to merge 41 commits into from
Closed

Rtd extend bug fix #5651

wants to merge 41 commits into from

Conversation

omerDotan
Copy link
Contributor

@omerDotan omerDotan commented Aug 23, 2020

Type of change

  • Bugfix

Description of change

bugfix for - #5519

the initSubModules function was called too late, causing the process not to work well with auction delay set (submodules was initiated after the auction)
changed the hook for the function call so the flow will work as expected

browsi sub module for real time data,
new hook bidsBackCallback,
fix for config unsubscribe
configure submodule on submodules.json
browsi sub module for real time data,
new hook bidsBackCallback,
fix for config unsubscribe
configure submodule on submodules.json
# Conflicts:
#	modules/browsiRtdProvider.js
# Conflicts:
#	modules/browsiRtdProvider.js
#	modules/rtdModule/index.js
@mike-chowla mike-chowla added needs review needs 2nd review Core module updates require two approvals from the core team labels Aug 26, 2020
@lgtm-com
Copy link

lgtm-com bot commented Aug 26, 2020

This pull request fixes 1 alert when merging 1fb1f31 into d38b5d0 - view on LGTM.com

fixed alerts:

  • 1 for Implicit operand conversion

@omerDotan
Copy link
Contributor Author

added changes as requested here -
prebid/prebid.github.io#2268 (comment)
(remove auction delay and related hook, change the name of getData function)

@bretg
Copy link
Collaborator

bretg commented Aug 28, 2020

There's a note in the docs PR prebid/prebid.github.io#2268 that concerns me about RTD-core:

Keep in mind it makes the auctionDelay param useless - meaning we will never delay the auction.
If the provider (submodule) will want to update the bidRequest with data that is not available immediately (e.g AJAX), he will have trouble doing it.

Of course RTD modules need the option of delaying the auction. Are you saying that somehow "getData()" supported a delay but updateBidRequest() doesn't?

If that's the case, then we need to (quickly) re-think the interface -- it's not useful without the ability to delay. What are the options? I'm out of my league, but the obvious approach would seem to be replacing updateBidRequest and auctionInit functions similar to addTargeting().

The requirement is that the sub-module be able to

  • delay the auction for the allowed time
  • inspect and update the bidRequest
  • inspect and update the auction

@smenzer
Copy link
Collaborator

smenzer commented Sep 9, 2020

any updates on this one?

@bretg
Copy link
Collaborator

bretg commented Sep 10, 2020

@omerBrowsi - do we still need this one with the 'phase 3' design we agreed on today?

@omerDotan
Copy link
Contributor Author

@bretg No, I will add the relevant code to the next commit

@bretg
Copy link
Collaborator

bretg commented Sep 14, 2020

Ok - will close this one then Thanks.

@bretg bretg closed this Sep 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review needs 2nd review Core module updates require two approvals from the core team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants