Our dt_from_string regex for time is optional.. as such we allow for non-matches.. but we don't check that the string as a whole matches a valid date/datetime string.. thus we don't fail when we should.
Created attachment 127246 [details] [review] Bug 29403: Unit tests Add a unit test for proper iso date handling
Created attachment 127247 [details] [review] Bug 29403: Match the whole string This patch wraps the constructed regex such that we expect the entire regex to match and don't allow leading or trailing garbage.
This should be tested in conjunction with the existing API tests.. I'm betting by making this more strict we could be exposing a few issues ;)
Created attachment 127254 [details] [review] Bug 29403: Fix api/v1/patrons.t t::lib::Dates::compare expects either correct DateTime objects, or a system formatted datetime strings to be passed.. but as we're testing the API, we are consistently using RFC3339 strings for date-times and ISO8601 strings for dates. We need to therefore pre-process the strings into DateTime objects for the comparison.
Created attachment 128076 [details] [review] Bug 29403: Unit tests Add a unit test for proper iso date handling Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 128077 [details] [review] Bug 29403: Match the whole string This patch wraps the constructed regex such that we expect the entire regex to match and don't allow leading or trailing garbage. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 128078 [details] [review] Bug 29403: Fix api/v1/patrons.t t::lib::Dates::compare expects either correct DateTime objects, or a system formatted datetime strings to be passed.. but as we're testing the API, we are consistently using RFC3339 strings for date-times and ISO8601 strings for dates. We need to therefore pre-process the strings into DateTime objects for the comparison. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Thanks for testing and SO :)
Created attachment 128272 [details] [review] Bug 29403: Unit tests Add a unit test for proper iso date handling Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 128273 [details] [review] Bug 29403: Match the whole string This patch wraps the constructed regex such that we expect the entire regex to match and don't allow leading or trailing garbage. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 128274 [details] [review] Bug 29403: Fix api/v1/patrons.t t::lib::Dates::compare expects either correct DateTime objects, or a system formatted datetime strings to be passed.. but as we're testing the API, we are consistently using RFC3339 strings for date-times and ISO8601 strings for dates. We need to therefore pre-process the strings into DateTime objects for the comparison. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 128450 [details] [review] Bug 29403: Unit tests Add a unit test for proper iso date handling Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 128451 [details] [review] Bug 29403: Match the whole string This patch wraps the constructed regex such that we expect the entire regex to match and don't allow leading or trailing garbage. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 128452 [details] [review] Bug 29403: Fix api/v1/patrons.t t::lib::Dates::compare expects either correct DateTime objects, or a system formatted datetime strings to be passed.. but as we're testing the API, we are consistently using RFC3339 strings for date-times and ISO8601 strings for dates. We need to therefore pre-process the strings into DateTime objects for the comparison. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In my opinion we should not backport those patches.
Pushed to master for 22.05, thanks to everybody involved [U+1F984]
(In reply to Fridolin Somers from comment #16) > Pushed to master for 22.05, thanks to everybody involved [U+1F984] This is causing failures on t/db_dependent/Circulation.t not ok 17 - CanBookBeRenewed tests # Failed test 'CanBookBeRenewed tests' # at t/db_dependent/Circulation.t line 1450. The given date (2021-12-15T16:06:30) does not match the date format (us) at /kohadevbox/koha/Koha/DateUtils.pm line 197. # Looks like your test exited with 11 just after 17. Dubious, test returned 11 (wstat 2816, 0xb00)
O crap, I see it Looks like it comes from test on no_auto_renewal_after_hard_limit
I revert from master
(In reply to Fridolin Somers from comment #19) > I revert from master This bug made things stricter. It highlighted we have fragile spots when it comes to date handling. It is not a bad thing per se on the master branch. But it is not bad to revert anyway. There are bugs in which Jonathan proposed alternative solutions
(In reply to Fridolin Somers from comment #19) > I revert from master Ideally we prefer to not revert but fix. It will get messy if you revert every time jenkins is failing after a push :)
(In reply to Nick Clemens from comment #17) > (In reply to Fridolin Somers from comment #16) > > Pushed to master for 22.05, thanks to everybody involved [U+1F984] > > This is causing failures on t/db_dependent/Circulation.t > > not ok 17 - CanBookBeRenewed tests > # Failed test 'CanBookBeRenewed tests' > # at t/db_dependent/Circulation.t line 1450. > The given date (2021-12-15T16:06:30) does not match the date format (us) at > /kohadevbox/koha/Koha/DateUtils.pm line 197. > # Looks like your test exited with 11 just after 17. > Dubious, test returned 11 (wstat 2816, 0xb00) This one is fun. We are storing a DT into a varchar (rule_value) and so the DT object is stringified using ISO 8601. And yes, this is closely related to bug 29718.
I had a feeling this might throw a few issues.. I kinda hoped to have it pushed and help deal with the repercussions.... or that we'd team together to try and spot repercussions and fix them before the push. Honestly.. I thought I'd run the whole test suit though, so that's my bad.. I should have got the tests all passing first.
Ah good points. I've dig about an hour trying to fix but couldn't get it work. > This one is fun. We are storing a DT into a varchar (rule_value) and so the DT object is stringified using ISO 8601. > And yes, this is closely related to bug 29718. Ah that is complex for me. We cant change how DT is stringified right ? So we better change how we store. Maybe a Koha::DateUtils dt_to_sql method ?
(In reply to Fridolin Somers from comment #24) > We cant change how DT is stringified right ? We can, this is the first option described on bug 29718. I don't think it's worth it. > So we better change how we store. > Maybe a Koha::DateUtils dt_to_sql method ? Hum? I am not sure I understand that part. This is DBMS internals, we cannot modify how we store date/datetime/timestamp.
(In reply to Jonathan Druart from comment #25) > > > So we better change how we store. > > Maybe a Koha::DateUtils dt_to_sql method ? > > Hum? I am not sure I understand that part. This is DBMS internals, we cannot > modify how we store date/datetime/timestamp. I mean a method to stringify a DT into a string that we store in database. Called "dt_to_sql" as oposite of "dt_from_string($str, 'sql')". We may call it "dt_to_string".
We don't need a to_sql_string.. that's handled already for db date/datetime/timestamp type fields.. in this case we're not dealing with such a field, we're dealing with a varchar.. which allows any ol' string. Does bug 29718 not basically resolve the test failure this introduces?
(In reply to Martin Renvoize from comment #27) > Does bug 29718 not basically resolve the test failure this introduces? I'll test. In error we see : The given date (2021-12-15T16:06:30) does not match the date format (us) Realy strange to see US format there, looks like its a call on "dt_from_string" without explicit format.
OK, I'll take a deeper look at the code to see what it's doing then.
OK.. I took a deeper look and my assumptions were correct.. so I applied the two patchsets and ran the tests again.. and for me.. they pass as expected?! Are you sure you had both this patchset and bug 29718 applied? It is a bit of a funky test... in smart-rules we actually grab the passed date, convert to a datetime, then convert to an iso string and drop the time component. In the test, we're just passing a DateTime object, so it'll include a time element and it'll contextually stringify using the datetime defaults (to ISO including Time - Hense the 'T'). What is happening in our DateUtils is that we try to match on preference format ('us' in this case which will fail) and then fallback to 'sql/iso'.
Created attachment 128825 [details] [review] Bug 29403: Fix unit test for bug 29718 Including a 'T' is actually valid for ISO date format, I was being naughty and writing the test to follow our code rather than writing the test to conform to the ISO spec first time around. Now we are properly supporting 'T' in ISO dates with bug 29718, we need to update the test here to properly reflect an invalid ISO date string.
Created attachment 128826 [details] [review] Bug 29403: Unit tests Add a unit test for proper iso date handling Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 128827 [details] [review] Bug 29403: Match the whole string This patch wraps the constructed regex such that we expect the entire regex to match and don't allow leading or trailing garbage. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 128828 [details] [review] Bug 29403: Fix api/v1/patrons.t t::lib::Dates::compare expects either correct DateTime objects, or a system formatted datetime strings to be passed.. but as we're testing the API, we are consistently using RFC3339 strings for date-times and ISO8601 strings for dates. We need to therefore pre-process the strings into DateTime objects for the comparison. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 128829 [details] [review] Bug 29403: Fix unit test for bug 29718 Including a 'T' is actually valid for ISO date format, I was being naughty and writing the test to follow our code rather than writing the test to conform to the ISO spec first time around. Now we are properly supporting 'T' in ISO dates with bug 29718, we need to update the test here to properly reflect an invalid ISO date string.
I reversed the dependencies and fixed a badly written test I included here in the first place.
Created attachment 129373 [details] [review] Bug 29403: Unit tests Add a unit test for proper iso date handling Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 129374 [details] [review] Bug 29403: Match the whole string This patch wraps the constructed regex such that we expect the entire regex to match and don't allow leading or trailing garbage. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 129375 [details] [review] Bug 29403: Fix api/v1/patrons.t t::lib::Dates::compare expects either correct DateTime objects, or a system formatted datetime strings to be passed.. but as we're testing the API, we are consistently using RFC3339 strings for date-times and ISO8601 strings for dates. We need to therefore pre-process the strings into DateTime objects for the comparison. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 129376 [details] [review] Bug 29403: Fix unit test for bug 29718 Including a 'T' is actually valid for ISO date format, I was being naughty and writing the test to follow our code rather than writing the test to conform to the ISO spec first time around. Now we are properly supporting 'T' in ISO dates with bug 29718, we need to update the test here to properly reflect an invalid ISO date string.
Created attachment 129391 [details] [review] Bug 29403: Unit tests Add a unit test for proper iso date handling Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 129392 [details] [review] Bug 29403: Match the whole string This patch wraps the constructed regex such that we expect the entire regex to match and don't allow leading or trailing garbage. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 129393 [details] [review] Bug 29403: Fix api/v1/patrons.t t::lib::Dates::compare expects either correct DateTime objects, or a system formatted datetime strings to be passed.. but as we're testing the API, we are consistently using RFC3339 strings for date-times and ISO8601 strings for dates. We need to therefore pre-process the strings into DateTime objects for the comparison. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 129394 [details] [review] Bug 29403: Fix unit test for bug 29718 Including a 'T' is actually valid for ISO date format, I was being naughty and writing the test to follow our code rather than writing the test to conform to the ISO spec first time around. Now we are properly supporting 'T' in ISO dates with bug 29718, we need to update the test here to properly reflect an invalid ISO date string. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Just a minor rebase to fix the test count
QA: Having a look here now
(In reply to Martin Renvoize from comment #0) > Our dt_from_string regex for time is optional.. as such we allow for > non-matches.. but we don't check that the string as a whole matches a valid > date/datetime string.. thus we don't fail when we should. A first observation: The bug title is not matching this description. This description is much wider than the title is suggesting.
- t::lib::Dates::compare( $updated_on_got, $updated_on_expected, 'updated_on values matched' ); + t::lib::Dates::compare( + dt_from_string( $updated_on_got, 'rfc3339' ), + dt_from_string( $updated_on_expected, 'rfc3339' ), + 'updated_on values matched' + ); But what does t::lib::Dates? sub compare { my ( $got, $expected ) = @_; my $dt_got = dt_from_string($got); my $dt_expected = dt_from_string($expected); my $diff = $dt_got->epoch - $dt_expected->epoch; if ( abs($diff) <= 5 ) { return 0 } return $diff > 0 ? 1 : -1; } So $got en $expected should be strings, right? Does this actually show that we miss a parameter in sub compare, and we are solving it by twisting the test instead?
(In reply to Jonathan Druart from comment #21) > (In reply to Fridolin Somers from comment #19) > > I revert from master > > Ideally we prefer to not revert but fix. It will get messy if you revert > every time jenkins is failing after a push :) Sure !
(In reply to Jonathan Druart from comment #15) > In my opinion we should not backport those patches. Seems to be wise. Will mark this as enh.
t::lib::Dates::compare expects either correct DateTime objects, or a system formatted datetime strings to be passed.. but as we're testing the API, we are consistently using RFC3339 strings for date-times and ISO8601 strings for dates. We need to therefore pre-process the strings into DateTime objects for the comparison. See comment48 Imo this sub needs more attention.
commit dfcb2b8071582a55e35c47926d4b5c1b8b2a9a59 Author: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Date: Fri Aug 25 17:56:52 2017 -0300 Bug 19176: Compare the number of seconds when comparing dates in tests Five years go fast.
Including a 'T' is actually valid for ISO date format, I was being naughty and writing the test to follow our code rather than writing the test to conform to the ISO spec first time around. Now we are properly supporting 'T' in ISO dates with bug 29718, we need to update the test here to properly reflect an invalid ISO date string. - $dt0 = dt_from_string('2021-11-03T10:16:59+00:00', 'iso'); + $dt0 = dt_from_string('2021-11-03T10:16:59Z+00:00', 'iso'); This is rather unclear. The changes do not reflect the description. You dont do anything with T here. Adding the Z is making the test pass by failure on the combination of Z and +00. I will add a comment on this line and another one to clarify.
Created attachment 129473 [details] [review] Bug 29403: (QA follow-up) Add comments why test should fail Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Setting the status to FQA for now. Please adjust or provide feedback on earlier comments. I have various tests failing but could not quickly decide if that relates to this patch set. Please provide evidence that all tests using dt_from_string still pass. Thanks!
* I've adapted the bug title as requested. * Before the last submission, I ran the test suite for db_dependant/api/* and everything is passing. I also ran a few other tests that included dt_from_string.. but not all.. nothing failed so far * The compare function is indeed a bit of a fudge.. but one from quite a while ago.. I'm not sure if we should be attempting to fix that here or not.. I'm also not entirely happy with the shared ISO/SQL/Fallback behaviour.. we're still not being explicit in our iso support (We're allowing for am/pm which is not allowed in ISO). Not sure what more to do here.. happy to discus for clarifications
OK, Running the whole test suite against this now.. will report back
(In reply to Marcel de Rooy from comment #48) > - t::lib::Dates::compare( $updated_on_got, $updated_on_expected, > 'updated_on values matched' ); > + t::lib::Dates::compare( > + dt_from_string( $updated_on_got, 'rfc3339' ), > + dt_from_string( $updated_on_expected, 'rfc3339' ), > + 'updated_on values matched' > + ); > > But what does t::lib::Dates? > > sub compare { > my ( $got, $expected ) = @_; > my $dt_got = dt_from_string($got); > my $dt_expected = dt_from_string($expected); > my $diff = $dt_got->epoch - $dt_expected->epoch; > if ( abs($diff) <= 5 ) { return 0 } > return $diff > 0 ? 1 : -1; > } > > So $got en $expected should be strings, right? > Does this actually show that we miss a parameter in sub compare, and we are > solving it by twisting the test instead? Will fix on bug 29884.
Created attachment 129486 [details] [review] Bug 29403: Fix api/v1/patrons.t t::lib::Dates::compare expects either correct DateTime objects, or a system formatted datetime strings to be passed.. but as we're testing the API, we are consistently using RFC3339 strings for date-times and ISO8601 strings for dates. We need to therefore pre-process the strings into DateTime objects for the comparison. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 129487 [details] [review] Bug 29403: Fix unit test for bug 29718 Including a 'T' is actually valid for ISO date format, I was being naughty and writing the test to follow our code rather than writing the test to conform to the ISO spec first time around. Now we are properly supporting 'T' in ISO dates with bug 29718, we need to update the test here to properly reflect an invalid ISO date string. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 129488 [details] [review] Bug 29403: (QA follow-up) Add comments why test should fail Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Jonathan Druart from comment #58) > (In reply to Marcel de Rooy from comment #48) > > - t::lib::Dates::compare( $updated_on_got, $updated_on_expected, > > 'updated_on values matched' ); > > + t::lib::Dates::compare( > > + dt_from_string( $updated_on_got, 'rfc3339' ), > > + dt_from_string( $updated_on_expected, 'rfc3339' ), > > + 'updated_on values matched' > > + ); > > > > But what does t::lib::Dates? > > > > sub compare { > > my ( $got, $expected ) = @_; > > my $dt_got = dt_from_string($got); > > my $dt_expected = dt_from_string($expected); > > my $diff = $dt_got->epoch - $dt_expected->epoch; > > if ( abs($diff) <= 5 ) { return 0 } > > return $diff > 0 ? 1 : -1; > > } > > > > So $got en $expected should be strings, right? > > Does this actually show that we miss a parameter in sub compare, and we are > > solving it by twisting the test instead? > > Will fix on bug 29884. That seems to be a good fix, but does not fully address my comment. This compare sub runs dt_from_string on its input. So normally I would expect it to have string parameters. The patrons.t test here is doing a dt conversion already with rfc3339. (Compare calls it again.) It would be more consistent if we would pass strings to compare and possibly a type like RFC3339, ISO8601, etc. I am seeing a hardcoded <=5, could be a parameter too or at least a constant? So yes, we are leaving scope here. But compare needs attention, and this patch set triggers it.
(In reply to Martin Renvoize from comment #57) > OK, Running the whole test suite against this now.. will report back Thanks. Any results?
(In reply to Marcel de Rooy from comment #62) > (In reply to Jonathan Druart from comment #58) > > (In reply to Marcel de Rooy from comment #48) > > > - t::lib::Dates::compare( $updated_on_got, $updated_on_expected, > > > 'updated_on values matched' ); > > > + t::lib::Dates::compare( > > > + dt_from_string( $updated_on_got, 'rfc3339' ), > > > + dt_from_string( $updated_on_expected, 'rfc3339' ), > > > + 'updated_on values matched' > > > + ); > > > > > > But what does t::lib::Dates? > > > > > > sub compare { > > > my ( $got, $expected ) = @_; > > > my $dt_got = dt_from_string($got); > > > my $dt_expected = dt_from_string($expected); > > > my $diff = $dt_got->epoch - $dt_expected->epoch; > > > if ( abs($diff) <= 5 ) { return 0 } > > > return $diff > 0 ? 1 : -1; > > > } > > > > > > So $got en $expected should be strings, right? > > > Does this actually show that we miss a parameter in sub compare, and we are > > > solving it by twisting the test instead? > > > > Will fix on bug 29884. > > That seems to be a good fix, but does not fully address my comment. This > compare sub runs dt_from_string on its input. So normally I would expect it > to have string parameters. The patrons.t test here is doing a dt conversion > already with rfc3339. (Compare calls it again.) It would be more consistent > if we would pass strings to compare and possibly a type like RFC3339, > ISO8601, etc. > I am seeing a hardcoded <=5, could be a parameter too or at least a constant? > So yes, we are leaving scope here. But compare needs attention, and this > patch set triggers it. It's not very nice I agree, but dt_from_string accepts DTs :)
'5' doesn't need to be a parameter. It's a global parameter for Jenkins, 1 and 2 was not enough, we push it to 5.
(In reply to Marcel de Rooy from comment #63) > (In reply to Martin Renvoize from comment #57) > > OK, Running the whole test suite against this now.. will report back > > Thanks. Any results? All passed, with the exception of a couple of conflict markers that were in a uncommited local file here.
Created attachment 129552 [details] [review] Bug 29403: Unit tests Add a unit test for proper iso date handling Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 129553 [details] [review] Bug 29403: Match the whole string This patch wraps the constructed regex such that we expect the entire regex to match and don't allow leading or trailing garbage. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 129554 [details] [review] Bug 29403: Fix api/v1/patrons.t t::lib::Dates::compare expects either correct DateTime objects, or a system formatted datetime strings to be passed.. but as we're testing the API, we are consistently using RFC3339 strings for date-times and ISO8601 strings for dates. We need to therefore pre-process the strings into DateTime objects for the comparison. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 129555 [details] [review] Bug 29403: Fix unit test for bug 29718 Including a 'T' is actually valid for ISO date format, I was being naughty and writing the test to follow our code rather than writing the test to conform to the ISO spec first time around. Now we are properly supporting 'T' in ISO dates with bug 29718, we need to update the test here to properly reflect an invalid ISO date string. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> The commit message is misleading here. Look at the Z in the code changes.
Created attachment 129556 [details] [review] Bug 29403: (QA follow-up) Add comments why test should fail Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Jonathan Druart from comment #65) > '5' doesn't need to be a parameter. It's a global parameter for Jenkins, 1 > and 2 was not enough, we push it to 5. Please cite me completely :) "Or a constant"? No big deal, but having this as a constant is surely better.
(In reply to Martin Renvoize from comment #66) > (In reply to Marcel de Rooy from comment #63) > > (In reply to Martin Renvoize from comment #57) > > > OK, Running the whole test suite against this now.. will report back > > > > Thanks. Any results? > > All passed, with the exception of a couple of conflict markers that were in > a uncommited local file here. Great. Confident enough to pass qa in view of responses etc.