[WHIT-2971] Add Topical Event to new document screen#11104
[WHIT-2971] Add Topical Event to new document screen#11104MuriloDalRi wants to merge 2 commits intomainfrom
Conversation
MuriloDalRi
force-pushed
the
topical-event-new-document-page
branch
from
2fdbc4b to
e07cb01
Compare
TonyGDS
left a comment
TonyGDS
left a comment
There was a problem hiding this comment.
I noticed that the topical event option hasn't been removed from the Standard Document menu. I think that is what is required in the ticket?:
1. Remove reference to the flexible content feature flag in the dropdown
lauraghiorghisor-tw
left a comment
lauraghiorghisor-tw
left a comment
There was a problem hiding this comment.
I'd make the changes in line with what you put in the PR description, i.e. actually turning the flag on for topical event. This should not be merged yet.
MuriloDalRi
force-pushed
the
topical-event-new-document-page
branch
from
e07cb01 to
f073dde
Compare
| } | ||
| types["topical_event"] = { | ||
| "klass" => StandardEdition, | ||
| "hint_text" => "Use this to create topical events.", |
There was a problem hiding this comment.
Can you please follow the pattern I have already linked to elsewhere, the one in the choose_type, and surface the topical_event json description as the hint text here?
There was a problem hiding this comment.
Just on this - if you mean this below @TonyGDS I think it will always show there, it's a Standard Edition. The already migrated types still show there too. Think it's fine. It might mean removing the flag altogether, but we still have history page there so not yet. Tagging @ChrisBAshton to clarify 😅 |
MuriloDalRi
force-pushed
the
topical-event-new-document-page
branch
from
f073dde to
ad6ff21
Compare
MuriloDalRi
force-pushed
the
topical-event-new-document-page
branch
from
ad6ff21 to
78356ce
Compare
lauraghiorghisor-tw
force-pushed
the
topical-event-new-document-page
branch
2 times, most recently
from
ed6617e to
9bc89b6
Compare
| types["topical_event"] = { | ||
| "klass" => StandardEdition, | ||
| "hint_text" => "Use this to create config-driven topical events.", | ||
| "hint_text" => ConfigurableDocumentType.find("topical_event").description, |
There was a problem hiding this comment.
As you can see in the last commit (and message) this line comes with a lot of changes in tests, though I would not say they are necessarily bad changes. We can alternatively just hard-code the thing.
There was a problem hiding this comment.
Should we also use the config for the label?
| @@ -0,0 +1,7 @@ | |||
| require_relative "mocha" | |||
|
|
|||
| Before do | |||
There was a problem hiding this comment.
Moving this setup into a helper is a nice touch. Capital B in before seems a bit weird for Ruby, but anyway, if it works
There was a problem hiding this comment.
Seems to be a cucumber hook in this context, fails with a b.
| types["topical_event"] = { | ||
| "klass" => StandardEdition, | ||
| "hint_text" => "Use this to create config-driven topical events.", | ||
| "hint_text" => ConfigurableDocumentType.find("topical_event").description, |
There was a problem hiding this comment.
Should we also use the config for the label?
lauraghiorghisor-tw
force-pushed
the
topical-event-new-document-page
branch
from
9bc89b6 to
428ae0a
Compare
lauraghiorghisor-tw
dismissed stale reviews from TonyGDS and themself
All concerns addressed.
lauraghiorghisor-tw
force-pushed
the
topical-event-new-document-page
branch
2 times, most recently
from
4249fbd to
1cb4748
Compare
We want to show a bit of hint text under the document type, which should come from the schema description, in the same way it does for other "single" document (sub)types such as news stories. The label should also come from the schema title. Topical events do not have subtypes so there is no additional choose type screen. Calling `ConfigurableDocumentType.find` with a "real" document type key put us in a position where we needed to either build a test type with that key or stub the call. I opted to do the latter, since we have generally kept our tests agnostic to any real type references.
TonyGDS
force-pushed
the
topical-event-new-document-page
branch
from
1cb4748 to
2bc571a
Compare
4 participants
Add “Topical Event” to the ‘New document’ screen to switch on the feature for our beta users.
🚨 Do not merge yet.
Jira