The course reserves entry form shows an error message instead of a form field for the department if no departments are defined: "No DEPARTMENT authorised values found! Please create one or more authorised values with the category DEPARTMENT." If you can't fill out the form because there are no departments, there's no reason to display the form. Otherwise people might fill in the rest of the fields and find they can't submit the form. It would be better to show the error message alone on the page and link the user to the authorized values page.
Created attachment 18671 [details] [review] Bug 10409 - Do not show course entry form if no departments are found The course reserves entry form should not be shown if there are no DEPARTMENT authorized values, since this prevents the form from being submitted. This patch replaces the form with an error message when no DEPARTMENT authorized values are found. Also corrected: - Corrected grid structure for more standard display - Converted labels with no corresponding inputs to <span class="label"> - Closed unclosed tags - Corrected incorrect capitalization This patch contains whitespace changes, so please ignore whitespace when examining changes. To test, delete any DEPARTMENT authorized values, if present. Create a new course in Course Reserves. You should see a warning that no DEPARTMENT values were found. If you are logged in with the correct permission, the warning should contain a link to the correct authorized value page. If you do no, the warning should refer the problem to an administrator. After creating one or more DEPARTMENT values, the form should display and submit correctly.
Created attachment 18734 [details] [review] Bug 10409 - Do not show course entry form if no departments are found The course reserves entry form should not be shown if there are no DEPARTMENT authorized values, since this prevents the form from being submitted. This patch replaces the form with an error message when no DEPARTMENT authorized values are found. Also corrected: - Corrected grid structure for more standard display - Converted labels with no corresponding inputs to <span class="label"> - Closed unclosed tags - Corrected incorrect capitalization This patch contains whitespace changes, so please ignore whitespace when examining changes. To test, delete any DEPARTMENT authorized values, if present. Create a new course in Course Reserves. You should see a warning that no DEPARTMENT values were found. If you are logged in with the correct permission, the warning should contain a link to the correct authorized value page. If you do no, the warning should refer the problem to an administrator. After creating one or more DEPARTMENT values, the form should display and submit correctly. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> After this patch has been applied, I get an error message and no form if I don't have any DEPARTMENTs defined, which makes sense given that the form can't be submitted without a DEPARTMENT.
Created attachment 19278 [details] [review] Bug 10409 - Do not show course entry form if no departments are found The course reserves entry form should not be shown if there are no DEPARTMENT authorized values, since this prevents the form from being submitted. This patch replaces the form with an error message when no DEPARTMENT authorized values are found. Also corrected: - Corrected grid structure for more standard display - Converted labels with no corresponding inputs to <span class="label"> - Closed unclosed tags - Corrected incorrect capitalization This patch contains whitespace changes, so please ignore whitespace when examining changes. To test, delete any DEPARTMENT authorized values, if present. Create a new course in Course Reserves. You should see a warning that no DEPARTMENT values were found. If you are logged in with the correct permission, the warning should contain a link to the correct authorized value page. If you do no, the warning should refer the problem to an administrator. After creating one or more DEPARTMENT values, the form should display and submit correctly. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> After this patch has been applied, I get an error message and no form if I don't have any DEPARTMENTs defined, which makes sense given that the form can't be submitted without a DEPARTMENT.
Created attachment 19298 [details] [review] [PASSED QA] Bug 10409 - Do not show course entry form if no departments are found The course reserves entry form should not be shown if there are no DEPARTMENT authorized values, since this prevents the form from being submitted. This patch replaces the form with an error message when no DEPARTMENT authorized values are found. Also corrected: - Corrected grid structure for more standard display - Converted labels with no corresponding inputs to <span class="label"> - Closed unclosed tags - Corrected incorrect capitalization This patch contains whitespace changes, so please ignore whitespace when examining changes. To test, delete any DEPARTMENT authorized values, if present. Create a new course in Course Reserves. You should see a warning that no DEPARTMENT values were found. If you are logged in with the correct permission, the warning should contain a link to the correct authorized value page. If you do no, the warning should refer the problem to an administrator. After creating one or more DEPARTMENT values, the form should display and submit correctly. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> After this patch has been applied, I get an error message and no form if I don't have any DEPARTMENTs defined, which makes sense given that the form can't be submitted without a DEPARTMENT. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes all tests and QA script. You now see a useful error message, when no departments have been defined. Else you are taken to the correct form.
There are still some things that are a bit problematic: <legend>[% IF course_id %] Edit [% ELSE %] Create [% END %] course</legend> 'Lazy' constructs like that make it unnecessarily hard to translators, because grammar works different in different languages and having single strings like that in a predefined order is not nice. Even in German it does not really work.
Created attachment 19299 [details] [review] Bug 10409: Follow up - improving strings for easier translation Fixing this line: <legend>[% IF course_id %] Edit [% ELSE %] Create [% END %] course</legend> As grammar works different in different languages, having single strings like that in a predefined order makes having a nice translation unnecessarily hard. This will make it a little easier: <legend>[% IF course_id %]Edit course[% ELSE %]Create course[% END %]</legend>
(In reply to Owen Leonard from comment #0) > The course reserves entry form shows an error message instead of a form > field for the department if no departments are defined: "No DEPARTMENT > authorised values found! Please create one or more authorised values with > the category DEPARTMENT." A question about the premise of this patch, possibly for Kyle: is there a functional reason why setting a department is absolutely required? If so, why is courses.department a nullable column?
Setting to "In discussion" pending a response to the question posed in comment 7.
(In reply to Galen Charlton from comment #7) > (In reply to Owen Leonard from comment #0) > > The course reserves entry form shows an error message instead of a form > > field for the department if no departments are defined: "No DEPARTMENT > > authorised values found! Please create one or more authorised values with > > the category DEPARTMENT." > > A question about the premise of this patch, possibly for Kyle: is there a > functional reason why setting a department is absolutely required? If so, > why is courses.department a nullable column? There is no functional reason to require the setting of a department. It was merely a requirement of the specification I was following ( where the functionality was based on another implementation of course reserves which made Department a required field ). Making department an optional value would do no harm. Of course, the same could be said for the "Name" field, so it's more of a philosophical question than anything. I guess the question is "In practice, will anyone want to create courses *without* course departments?". So, we can either leave department as a required field, or make it optionally require via a system preference. What are your feelings on those options?
When we alrady discuss required fields... I wondered why the course number is required. I think the way course reserves work here, normally they just give the name of the professor or course without a number.
(In reply to Katrin Fischer from comment #10) > When we alrady discuss required fields... I wondered why the course number > is required. I think the way course reserves work here, normally they just > give the name of the professor or course without a number. Katrin, basically the same answer as before. It was in the spec, it doesn't *really* have to be requiired, and we could make it being a requirement optional via a system preference.
I think if changing the required fields is something that libraries want a new bug should be created. I don't think the discussion should hold up this patch.
Hm, still think it's a bit funny to have unrequired (by the system) fields required. I think the question here is if we push Owen's fix or remove the required attribute from the field?
(In reply to Katrin Fischer from comment #13) > Hm, still think it's a bit funny to have unrequired (by the system) fields > required. I think the question here is if we push Owen's fix or remove the > required attribute from the field? Owen's fix does not change the functionality of the system, whereas removing the required attribute would. I would suggest we push this along, and if you'd like to see the option to make any of the given field values required/optional, we should file a separate bug for that. It really has no bearing on this particular bug. Both pre and post patch behaviors prohibit the creation of a course without a department, and only the presentation has changed.
Created attachment 19683 [details] [review] Bug 10409: Follow up - improving strings for easier translation Fixing this line: <legend>[% IF course_id %] Edit [% ELSE %] Create [% END %] course</legend> As grammar works different in different languages, having single strings like that in a predefined order makes having a nice translation unnecessarily hard. This will make it a little easier: <legend>[% IF course_id %]Edit course[% ELSE %]Create course[% END %]</legend> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(In reply to Katrin Fischer from comment #13) > Hm, still think it's a bit funny to have unrequired (by the system) fields > required. I think the question here is if we push Owen's fix or remove the > required attribute from the field? I've gone ahead and pushed Owen's patch (thanks, Owen!) and your follow-up (thanks, Katrin!), but from my POV, the question of what fields should be required remains open, so I've opened bug 10603.