Convert Week Goodie to Structured Template, update tests #950
Conversation
|
@moollaza Will be good to see this finally released into the wild 🌵 One thing I've noticed is the use of the simple past tense began for dates in the future, this seems incorrect. Can a conditional statement to change wording be added? Edit: Actually I've just looked at the triggers and it's incorrect, "what was the 6th week of 2016" shouldn't trigger "what is the 6th week of 2016" should. Guessing that's what you mean by:
Current Suggestions: |
| return $response, | ||
| structured_answer => { | ||
| input => [], | ||
| operation => "Assuming the week starts on Monday", |
There was a problem hiding this comment.
We could support this as a mechanism if we just change structured answers to have "subtitle" instead of "operation" ?
There was a problem hiding this comment.
@mintsoft I agree, but on a very awesome note, we already have a working demo of Goodies passing along data to the frontend (in an almost identical fashion to Spice's passing an object to Spice.add())
Once that's available I suspect we'll be able to port Goodies to use that approach where we'd have "title", "subtitle" etc.
|
@MrChrisW really good find regarding the trigger/word tense. I'll see if I can fix that up |
|
@javathunderman I'll run through them to see if any need updating. We should be able to launch the ones that don't. |
|
Should be fixed now! |
|
@moollaza Great! |
|
@moollaza the tests are failing across the board on this PR, I'm guessing it's something else that's done it though? |
|
@MrChrisW Yikes. When is that going to be fixed? |
|
Should be fixed now, also added "when" to triggers so we can handle grammatically correct questions for future dates |
|
@moollaza Thanks for seeing this through. I have gotten pretty busy with school. Great job! :) |
|
@moollaza Tested and working! 👍 |
|
I wonder if we should add the end day too? Probably not important--I'm not the target audience of this goodie. :) Works pretty well! Fails when I put in "what was the 5th week of 0000", though:
|
|
Also fails when I put in "what was the 0 week of 2011":
|
|
Another one with "what was the 99th week of 2011"--sorry for all the comments! |
|
@jagtalon Damn nice catch! The |
|
@MrChrisW thanks! How do you know that we need to limit it to 1900 and 2099? It looks like it's working for the year 1100 for me (and correct too) |
|
@jagtalon I was just trying to think of a reasonable/useful range 😄 |
|
Seems like I have called attention to this. Mission accomplished. I'm going to work on some other goodies that have to be released. |
|
@moollaza IIRC there's already a regex for the years within the role; it probably just needs wrapping with a getter |
|
@moollaza Seems like this has taken a back seat- any update? |
|
Sorry guys, I've been busy dealing with other new IA's and bugs -- I'll get to this eventually. If anyone else wants to finish it up, you're welcome to do so! |
|
@moollaza I'll see if I can slot it in this weeked; I'll probably supplant some year validation into the DatesRole and then use it here |
|
ping @mintsoft |
|
@javathunderman Apologies; I've been moving house so I've not really had the time! Hopefully, now I'm all done I can dig something up. |
|
@mintsoft No problem! |
|
Highjacked in: #1348 |
Fixes #712.
My only thought is that I've kinda "abused" the
operationto provide a subtitle. I tried a few permutation where I properly indicated the operation and input but in every case it looked repetitive and ugly.//cc @jagtalon @mintsoft @gsquire @javathunderman
The triggering could be relaxed a bit, but I think that's an issue for another day. Someone from the community is welcome to tackle that ;)