Exclude featured experiment from home page list #3276
Conversation
meandavejustice
commented
Jan 23, 2018
meandavejustice
commented
- fixes Do not include featured experiment in homepage card lists #3244
meandavejustice
force-pushed
the
3244-featured-experiment-excluded-home-page
branch
from
1277864
to
43ac27b
Compare
|
I guess I didn't see this before opening my pr. Is there any reason to do it this way instead of just not featuring any experiment? |
|
@fzzzy This is a different issue, this makes it so we don't duplicate the featured experiment in the experiment card list. |
| const { experiments, isAfterCompletedDate, featuredExperiments } = this.props; | ||
| const { isAfterCompletedDate, featuredExperiments } = this.props; | ||
|
|
||
| const experiments = this.props.experiments.filter(x => !x.is_featured); |
| len(data['results']) - completed_experiments - 1) | ||
| else: | ||
| assert len(page.body.experiments) == int( | ||
| len(data['results']) - completed_experiments) |
There was a problem hiding this comment.
Can you add another assertion that the featured experiment specifically is not in the list of experiments?
There was a problem hiding this comment.
filed #3295, I still can't get integration tests running on my machine, it looks to be because of tox-dev/tox#121. I can loop back around to get this test written when I'm back at my house and on my linux machine.
|
Moved filtering into selector, added unit test, and fixed exiting tests to work with change. filed #3295 as a reminder to fix when I have access to my linux machine since tox-dev/tox#121 is causing failures on my windows laptop. |
meandavejustice
force-pushed
the
3244-featured-experiment-excluded-home-page
branch
from
900eea3
to
b7c8536
Compare
|
@meandavejustice Hmm, cloned this branch locally and did a bit of testing on it. Not sure this is working as intended since I'm seeing also the "Cliqz" experiment while my browser is [en-US] localized. Maybe the change affected the localization restrictions? |
|
This generally looks good, but I'm sitting on approving until the issue Paul raised is addressed. |
|
@chuckharmston I'll try and get to paul's changes tomorrow morning, I'll ping ya when they're in |
meandavejustice
force-pushed
the
3244-featured-experiment-excluded-home-page
branch
from
b7c8536
to
d9833bd
Compare
meandavejustice
force-pushed
the
3244-featured-experiment-excluded-home-page
branch
from
d9833bd
to
7f93d4b
Compare
meandavejustice
force-pushed
the
3244-featured-experiment-excluded-home-page
branch
from
7f93d4b
to
042ba49
Compare
meandavejustice
deleted the
3244-featured-experiment-excluded-home-page
branch
