Bug 26055

I could work on this one, but I wonder about the best way to handle this.

In Koha, we typically do not change existing behaviour, so we add system preferences. 

I'm undecided whether the current behaviour is a "bug" or if it's just a system design with which I disagree. 

Perhaps the solution is to create a new option in the editor and use a syspref to change the button.

Note the following:

1) /cgi-bin/koha/cataloguing/editor.pl#new/
This only shows fields that are marked as "Mandatory"

2) /cgi-bin/koha/cataloguing/editor.pl#new-full/
This shows all fields in the framework regardless of "Mandatory" or "visibility" states

Relevant code:

./koha-tmpl/intranet-tmpl/prog/en/includes/cateditor-ui.inc
'new': {
            titleForRecord: _("Editing new record"),
            get: function( id, callback ) {
                record = new MARC.Record();
                KohaBackend.FillRecord( '', record );

                callback( record );
            },
        },

"koha-tmpl/intranet-tmpl/lib/koha/cateditor/koha-backend.js"
FillRecord: function( frameworkcode, record, allTags ) {
            $.each( _frameworks[frameworkcode], function( undef, tag ) {
                var tagnum = tag[0], taginfo = tag[1];

                if ( taginfo.mandatory != "1" && !allTags ) return;

                var fields = record.fields(tagnum);

                if ( fields.length == 0 ) {
                    var newField = new MARC.Field( tagnum, ' ', ' ', [] );
                    fields.push( newField );
                    record.addFieldGrouped( newField );

                    if ( tagnum < '010' ) {
                        newField.addSubfield( [ '@', (taginfo.subfields[0] ? taginfo.subfields[0][1].defaultvalue : null ) || '' ] );
                        return;
                    }
                }

                $.each( taginfo.subfields, function( undef, subfield ) {
                    var subfieldcode = subfield[0], subfieldinfo = subfield[1];

                    if ( subfieldinfo.mandatory != "1" && !subfieldinfo.defaultvalue && !allTags ) return;

                    $.each( fields, function( undef, field ) {
                        if ( !field.hasSubfield(subfieldcode) ) {
                            field.addSubfieldGrouped( [ subfieldcode, subfieldinfo.defaultvalue || '' ] );
                        } else if ( subfieldinfo.defaultvalue && field.subfield( subfieldcode ) === '' ) {
                            field.subfield( subfieldcode, subfieldinfo.defaultvalue );
                        }
                    } );
                } );
            } );
        },

I don't like the idea of making Visibility apply to Rancor. In the regular editor, visibility is determining whether or not a tag is available to put a value into at all. That really doesn't apply in Rancor, where one can manually key in any tag one wants. I suppose we could add a new checkbox for Advanced Editor to the visibility options.
This highlights a broader issue of how Rancor's relationship to frameworks is confusingly vague compared to the regular editor's relationship to frameworks.

(In reply to Andrew Fuerste-Henry from comment #3)
> I don't like the idea of making Visibility apply to Rancor. In the regular
> editor, visibility is determining whether or not a tag is available to put a
> value into at all. That really doesn't apply in Rancor, where one can
> manually key in any tag one wants. I suppose we could add a new checkbox for
> Advanced Editor to the visibility options.
> This highlights a broader issue of how Rancor's relationship to frameworks
> is confusingly vague compared to the regular editor's relationship to
> frameworks.

+1 

I agree that I would not like seeing the editor visibility used for both editors. Libraries might use both and while you HAVE to check visibility to be able to enter it at all in one editor, it would get in the way in the other. You will have to delete empty fields before saving, so you will want to keep it to an absolute minimum set = different use case. I agree a new checkbox would be a good option and relabelling "editor" to make clear what applies to which.

(In reply to Andrew Fuerste-Henry from comment #3)
> This highlights a broader issue of how Rancor's relationship to frameworks
> is confusingly vague compared to the regular editor's relationship to
> frameworks.

Agreed

(In reply to Katrin Fischer from comment #4)
> I agree a new
> checkbox would be a good option and relabelling "editor" to make clear what
> applies to which.

While a new checkbox sounds like a good idea, it is easier said than done. 

Here's some comments on the current mapping from ./Koha/Filter/MARC/ViewPolicy.pm:

# marc_subfields_structure.hidden
# allows you to select from 19 possible visibility conditions, 17 of which are implemented. They are the following:
# -9 => Future use
# -8 => Flag
# -7 => OPAC !Intranet !Editor Collapsed
# -6 => OPAC Intranet !Editor !Collapsed
# -5 => OPAC Intranet !Editor Collapsed
# -4 => OPAC !Intranet !Editor !Collapsed
# -3 => OPAC !Intranet Editor Collapsed
# -2 => OPAC !Intranet Editor !Collapsed
# -1 => OPAC Intranet Editor Collapsed
# 0 => OPAC Intranet Editor !Collapsed
# 1 => !OPAC Intranet Editor Collapsed
# 2 => !OPAC !Intranet Editor !Collapsed
# 3 => !OPAC !Intranet Editor Collapsed
# 4 => !OPAC Intranet Editor !Collapsed
# 5 => !OPAC !Intranet !Editor Collapsed
# 6 => !OPAC Intranet !Editor !Collapsed
# 7 => !OPAC Intranet !Editor Collapsed
# 8 => !OPAC !Intranet !Editor !Collapsed
# 9 => Future use
# ( ! means 'not visible' or in the case of Collapsed 'not Collapsed')

Firstly, the premise that there are 19 possible visibility conditions is false. While the "hidden" field is TINYINT(1), the 1 apparently refers to column padding rather than allowed characters. TINYINT(1) actually allows for values from -128 to +127. 

Secondly, in theory, 4 options means 16 permutations and 5 options means 25 permutations, but I doubt we want to include the new Rancor option in any of the existing 16 permutations, so I think we end up with 32 permutations like below? (Flag not being included in these counts obviously.)

# -16 => OPAC Intranet Editor Collapsed Rancor
# -15 => OPAC Intranet Editor !Collapsed Rancor
# -14 => OPAC Intranet !Editor Collapsed Rancor
# -13 => OPAC Intranet !Editor !Collapsed Rancor
# -12 => OPAC !Intranet Editor Collapsed Rancor
# -11 => OPAC !Intranet Editor !Collapsed Rancor
# -10 => OPAC !Intranet !Editor Collapsed Rancor
# -9 =>  OPAC !Intranet !Editor !Collapsed Rancor

# -8 =>  Flag
# -7 =>  OPAC !Intranet !Editor Collapsed !Rancor
# -6 =>  OPAC Intranet !Editor !Collapsed !Rancor
# -5 =>  OPAC Intranet !Editor Collapsed !Rancor
# -4 =>  OPAC !Intranet !Editor !Collapsed !Rancor
# -3 =>  OPAC !Intranet Editor Collapsed !Rancor
# -2 =>  OPAC !Intranet Editor !Collapsed !Rancor
# -1 =>  OPAC Intranet Editor Collapsed !Rancor
# 0 =>   OPAC Intranet Editor !Collapsed !Rancor
# 1 =>  !OPAC Intranet Editor Collapsed !Rancor
# 2 =>  !OPAC !Intranet Editor !Collapsed !Rancor
# 3 =>  !OPAC !Intranet Editor Collapsed !Rancor
# 4 =>  !OPAC Intranet Editor !Collapsed !Rancor
# 5 =>  !OPAC !Intranet !Editor Collapsed !Rancor
# 6 =>  !OPAC Intranet !Editor !Collapsed !Rancor
# 7 =>  !OPAC Intranet !Editor Collapsed !Rancor
# 8 =>  !OPAC !Intranet !Editor !Collapsed !Rancor

# 9 =>  !OPAC !Intranet !Editor !Collapsed Rancor
# 10 => !OPAC !Intranet !Editor Collapsed Rancor
# 11 => !OPAC !Intranet Editor !Collapsed Rancor
# 12 => !OPAC !Intranet Editor Collapsed Rancor
# 13 => !OPAC Intranet !Editor !Collapsed Rancor
# 14 => !OPAC Intranet !Editor Collapsed Rancor
# 15 => !OPAC Intranet Editor !Collapsed Rancor
# 16 => !OPAC Intranet Editor Collapsed Rancor

It's a shame we didn't go with bitpacking for the "hidden" field. That would've been so much easier. Or just had separate database fields. 

Does that look right to other people?

Alternatively... it looks like we no longer support SQL export/import... so maybe just adding a separate column for "show_advanced_editor" might be better.

I'd actually use a new database column instead of adding to the hard to understand number scheme.

(In reply to Katrin Fischer from comment #7)
> I'd actually use a new database column instead of adding to the hard to
> understand number scheme.

I was concerned about this breaking import/export of frameworks, but I think it might be OK. I'll have to look into it a bit further at some point.