Checklist: Hide banner from too old sites. #26867
Conversation
1a98fd6 to
76783d8
Compare
|
Rebased |
76783d8 to
f564eea
Compare
client/state/sites/constants.js
Outdated
| 'jetpack_version', | ||
| 'main_network_site', | ||
| 'onboarding_checklist_initialized', |
There was a problem hiding this comment.
Do we really need this off the site object? Please be careful about bloating the site object further. We do have the option of adding additional endpoints if necessary.
Do you mind expanding on what this prop means? It still looks like the client is performing a number of checks in addition to looking at this value.
There was a problem hiding this comment.
It basically means that the checklist has been initialized on the server side. I think that in practice it should be pretty redundant with the site age check already happening in the selector so I'll just eliminate it as I'd like to get this out the door.
There was a problem hiding this comment.
Thanks for the extra context @mattwiebe !
redundant with the site age check already happening in the selector
|
@damiannep this will solve that issue |
|
Did we explore performing this login in the backend? Do we not already poll for the checklist for a site? If the site is ineligible or too old what would prevent us from simply returning a response that indicates as much? We could keep this code out of Calypso which would help guard against bloat and we could make sure that the mobile teams wouldn't have to recreate the logic and keep it in sync on their end. |
Changed in #26867, regression introduced during rebase.
The onboarding checklist may not work properly for the sites created before the feature was launched due to lack of
onboarding_checklist_initializedoption that is generated when a site is created. This PR will check if the site is eligible for the checklist and hide the banner if not.Note: You need to apply D17548-code to your sandbox first.
Test plan