Bug 23849 - Update the list of sysprefs to share with Hea
Summary: Update the list of sysprefs to share with Hea
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Arthur Suzuki
QA Contact: Testopia
URL:
Keywords:
: 22546 36210 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-10-21 09:03 UTC by Jonathan Druart
Modified: 2024-12-04 13:33 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 23849: [DO NOT PUSH] list of sysprefs in DB (15.06 KB, patch)
2019-10-21 09:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23849: [DO NOT PUSH] list of sysprefs already shared (5.36 KB, patch)
2019-10-21 09:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23849: [DO NOT PUSH] List of sysprefs we share but not exist in DB (685 bytes, patch)
2019-10-21 09:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23849: [DO NOT PUSH] Not in DB - comments (939 bytes, patch)
2019-10-21 09:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23849: [DO NOT PUSH] List of sysprefs in DB we do not share (10.45 KB, patch)
2019-10-21 09:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23849: [DO NOT PUSH] List of sysprefs in DB we do not share - comments (11.05 KB, patch)
2019-10-21 09:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23849: Update the list of syspref sent to HEA (57.65 KB, patch)
2024-11-27 14:31 UTC, Arthur Suzuki
Details | Diff | Splinter Review
Bug 23849: (QA follow-up) remove duplicates on deprecated shared sysprefs (4.88 KB, patch)
2024-12-04 13:27 UTC, Arthur Suzuki
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2019-10-21 09:03:45 UTC
We have a lot of sysprefs that are not recorded in Hea. We lack a process to keep the list up-to-date.

We are going to have an updated list for 19.11. For next releases it should be a new post release task (https://wiki.koha-community.org/wiki/Post_major_release_tasks).

This bug report could be reused for the next releases.
Comment 1 Jonathan Druart 2019-10-21 09:03:58 UTC
*** Bug 22546 has been marked as a duplicate of this bug. ***
Comment 2 Jonathan Druart 2019-10-21 09:42:29 UTC
Created attachment 94424 [details] [review]
Bug 23849: [DO NOT PUSH] list of sysprefs in DB
Comment 3 Jonathan Druart 2019-10-21 09:42:32 UTC
Created attachment 94425 [details] [review]
Bug 23849: [DO NOT PUSH] list of sysprefs already shared
Comment 4 Jonathan Druart 2019-10-21 09:42:36 UTC
Created attachment 94426 [details] [review]
Bug 23849: [DO NOT PUSH] List of sysprefs we share but not exist in DB
Comment 5 Jonathan Druart 2019-10-21 09:42:39 UTC
Created attachment 94427 [details] [review]
Bug 23849: [DO NOT PUSH] Not in DB - comments
Comment 6 Jonathan Druart 2019-10-21 09:42:43 UTC
Created attachment 94428 [details] [review]
Bug 23849: [DO NOT PUSH] List of sysprefs in DB we do not share
Comment 7 Jonathan Druart 2019-10-21 09:42:46 UTC
Created attachment 94429 [details] [review]
Bug 23849: [DO NOT PUSH] List of sysprefs in DB we do not share - comments
Comment 8 Jonathan Druart 2019-10-21 09:44:59 UTC
I have generated:
- a list of sysprefs we already shared (in C4/UsageStats.pm) - file sorted_sysprefs_recorded
- a list of sysprefs we have in DB - file sorted_sysprefs

Then, using `comm`:
- a list of syspref we share but is not in DB - file shared_but_not_in_DB
- a list of syspref we do not share (yet?) - file in_DB_but_not_shared

I am going to commit and provide a patch, to help keeping track of the changes. The idea is to know which sysprefs we want to track with Hea.

Could you take a look (especially the file in_DB_but_not_shared) and comment please?
Basically we want to share the sysprefs that could be useful to track the usage, and that do not contain private data.
Comment 9 Jonathan Druart 2019-10-21 09:48:58 UTC
(In reply to Jonathan Druart from comment #0)
> We are going to have an updated list for 19.11. For next releases it should
> be a new post release task
> (https://wiki.koha-community.org/wiki/Post_major_release_tasks).

It should actually be a pre release task!
Comment 10 Martin Renvoize (ashimema) 2019-10-23 11:15:15 UTC
OK, I've gone through your lists and I agree with all your observations..

I was going to get Katrin to take a look too as she's a stickler for detail here :)
Comment 11 Paul Poulain 2019-10-25 08:17:00 UTC
Comment on attachment 94426 [details] [review]
Bug 23849: [DO NOT PUSH] List of sysprefs we share but not exist in DB

Good job Jonathan !
looking at this list, they used to exist, so there's a value on hea for most of them.
So the task is both remove them on Koha side, as well as on HEA

question: version, are you sure it doesn't exist ?
Comment 12 Paul Poulain 2019-10-25 08:27:20 UTC
Comment on attachment 94428 [details] [review]
Bug 23849: [DO NOT PUSH] List of sysprefs in DB we do not share

Looking at the list, I see that you made an SQL check.
There are some sysprefs that we will never share, and even some that we can't share. For example casServerUrl, or BakerTaylorPassword.
Where we wrote HEA, we selected only the ones we thought useful.
I agree we must update this list, but maybe by checking those who have been introduced since HEA was pushed (and if someone think an old one is missing, then we still can add it)
Comment 13 Jonathan Druart 2019-10-25 09:37:32 UTC
(In reply to Paul Poulain from comment #11)
> Comment on attachment 94426 [details] [review] [review]
> Bug 23849: [DO NOT PUSH] List of sysprefs we share but not exist in DB
> 
> Good job Jonathan !
> looking at this list, they used to exist, so there's a value on hea for most
> of them.
> So the task is both remove them on Koha side, as well as on HEA
> 
> question: version, are you sure it doesn't exist ?

it's because of "version" vs "Version".

(In reply to Paul Poulain from comment #12)
> Comment on attachment 94428 [details] [review] [review]
> Bug 23849: [DO NOT PUSH] List of sysprefs in DB we do not share
> 
> Looking at the list, I see that you made an SQL check.
> There are some sysprefs that we will never share, and even some that we
> can't share. For example casServerUrl, or BakerTaylorPassword.
> Where we wrote HEA, we selected only the ones we thought useful.
> I agree we must update this list, but maybe by checking those who have been
> introduced since HEA was pushed (and if someone think an old one is missing,
> then we still can add it)

Yes but it seems easier to redo the whole check and start again with a clean base.
See the version of the patches " - comments" to see the ones I have removed from the list.
Comment 14 Paul Poulain 2019-10-25 09:57:32 UTC
(In reply to Jonathan Druart from comment #13)
> (In reply to Paul Poulain from comment #11)
> > Comment on attachment 94426 [details] [review] [review] [review]
> > Bug 23849: [DO NOT PUSH] List of sysprefs we share but not exist in DB
> > 
> > Good job Jonathan !
> > looking at this list, they used to exist, so there's a value on hea for most
> > of them.
> > So the task is both remove them on Koha side, as well as on HEA
> > 
> > question: version, are you sure it doesn't exist ?
> 
> it's because of "version" vs "Version".
> 
> (In reply to Paul Poulain from comment #12)
> > Comment on attachment 94428 [details] [review] [review] [review]
> > Bug 23849: [DO NOT PUSH] List of sysprefs in DB we do not share
> > 
> > Looking at the list, I see that you made an SQL check.
> > There are some sysprefs that we will never share, and even some that we
> > can't share. For example casServerUrl, or BakerTaylorPassword.
> > Where we wrote HEA, we selected only the ones we thought useful.
> > I agree we must update this list, but maybe by checking those who have been
> > introduced since HEA was pushed (and if someone think an old one is missing,
> > then we still can add it)
> 
> Yes but it seems easier to redo the whole check and start again with a clean
> base.
> See the version of the patches " - comments" to see the ones I have removed
> from the list.

do you mean https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=94429&action=diff ?
I've read the diff, most of the lines have a "> no" => does it mean we wouldn't share them on HEA ? (if yes, I agree)
The only one that would be added is "AnonymousPatron" (and yes, I think it should be added)

Idea : what about adding some statistics like "intranetuserjs yes/no" => yes if there's something, no if it's empty ? (intranetuserjs, opacuserjs, opac & intranet css, opacheader, opacnav, opacbottom). For sysprefs *XSLT* => we could send "empty/default/local".
Comment 15 Jonathan Druart 2019-10-25 10:15:25 UTC
(In reply to Paul Poulain from comment #14)
> (In reply to Jonathan Druart from comment #13)
> > (In reply to Paul Poulain from comment #11)
> > > Comment on attachment 94426 [details] [review] [review] [review] [review]
> > > Bug 23849: [DO NOT PUSH] List of sysprefs we share but not exist in DB
> > > 
> > > Good job Jonathan !
> > > looking at this list, they used to exist, so there's a value on hea for most
> > > of them.
> > > So the task is both remove them on Koha side, as well as on HEA
> > > 
> > > question: version, are you sure it doesn't exist ?
> > 
> > it's because of "version" vs "Version".
> > 
> > (In reply to Paul Poulain from comment #12)
> > > Comment on attachment 94428 [details] [review] [review] [review] [review]
> > > Bug 23849: [DO NOT PUSH] List of sysprefs in DB we do not share
> > > 
> > > Looking at the list, I see that you made an SQL check.
> > > There are some sysprefs that we will never share, and even some that we
> > > can't share. For example casServerUrl, or BakerTaylorPassword.
> > > Where we wrote HEA, we selected only the ones we thought useful.
> > > I agree we must update this list, but maybe by checking those who have been
> > > introduced since HEA was pushed (and if someone think an old one is missing,
> > > then we still can add it)
> > 
> > Yes but it seems easier to redo the whole check and start again with a clean
> > base.
> > See the version of the patches " - comments" to see the ones I have removed
> > from the list.
> 
> do you mean
> https://bugs.koha-community.org/bugzilla3/attachment.
> cgi?id=94429&action=diff ?
> I've read the diff, most of the lines have a "> no" => does it mean we
> wouldn't share them on HEA ? (if yes, I agree)

Yes. Most important is to catch the ones I missed.

> The only one that would be added is "AnonymousPatron" (and yes, I think it
> should be added)

Indeed!

> Idea : what about adding some statistics like "intranetuserjs yes/no" => yes
> if there's something, no if it's empty ? (intranetuserjs, opacuserjs, opac &
> intranet css, opacheader, opacnav, opacbottom). For sysprefs *XSLT* => we
> could send "empty/default/local".

Good idea. Definitely for a separate bug report.
Comment 16 Paul Poulain 2019-10-25 12:43:54 UTC
see bug 23898
Comment 17 Victor Grousset/tuxayo 2020-05-06 17:57:14 UTC
After reviewing in a local Koha all the sysprefs of the file "in_DB_but_not_shared" here are some notes.

== Pretty sure they should be > No ==
Or just share if it's empty/default or custom.
AdlibrisCoversURL
EmailAddressForSuggestions
ILS-DI:AuthorizedIPs
NewItemsDefaultLocation
OpacHiddenItems
OpacHiddenItemsExceptions
ReplyToDefault
RestrictedPageLocalIPs
RestrictedPageTitle
ReturnpathDefault
SelfCheckAllowByIPRanges
UpdateItemLocationOnCheckin


== Maybe false positive, could be yes (share) instead of no ==
OverDrivePasswordRequired
OverDriveUsername
NovelistSelectStaffEnabled
NovelistSelectView
OAI-PMH:AutoUpdateSets
OAI-PMH:MaxCount
opaclanguages
OpacLocationOnDetail
OPACResultsLibrary
RESTOAuth2ClientCredentials
SyndeticsAuthorNotes
SyndeticsCoverImages
SyndeticsCoverImageSize
SyndeticsEditions
SyndeticsExcerpt
SyndeticsReviews
SyndeticsSeries
SyndeticsSummary
SyndeticsTOC


== We could just share if it's empty/default or custom ==
This list will be copied to bug 23898
AmazonAssocTag
IntranetCirculationHomeHTML
IntranetFavicon
IntranetmainUserblock
IntranetNav
IntranetReportsHomeHTML
IntranetSlipPrinterJS
intranetstylesheet
OpacCustomSearch
OPACHoldsIfAvailableAtPickupExceptions
OpacLoginInstructions
OpacMainUserBlock
OpacMoreSearches
OPACResultsSidebar
OPACSearchForTitleIn
ProcessingFeeNote
RestrictedPageContent
RoutingListNote
SpineLabelFormat
XSLTDetailsDisplay


== Not sure, does any know more about these sysprefs? ==
Can field lists be an issue? They might just be useless actually.
Or we could just want to know if it's empty/default or custom.
Precedent: NotesBlacklist is currently marked as > No
AuthoritySeparator
BibtexExportAdditionalFields
ExportRemoveFields
ISBD
MarcFieldsToOrder
MarcItemFieldsToOrder
MergeReportFields
OPACISBD
PatronQuickAddFields
RisExportAdditionalFields
StatisticsFields
SubfieldsToAllowForRestrictedBatchmod
SubfieldsToAllowForRestrictedEditing
SubfieldsToUseWhenPrefill
SubscriptionDuplicateDroppedInput

Similar doubts as previous paragraph:
ItemsDeniedRenewal
LinkerOptions
MARCAuthorityControlField008
UNIMARCAuthorityField100

Are backends lists an issue? Or is it actually very useful?
ILLOpacbackends

Could the directory name be private and thus not sharable?
intranet_includes

Could some codes be so rare that they are equivalent of sharing the library name?
MARCOrgCode

That should be ok right?
borrowerRelationship
BorrowersTitles


== We might want to filter the content of these ==
To only get Koha's official fields and not custom DB fields.
They could be ignored now and be in a follow up bug.
Or we could just want to know if it's empty/default or custom.
BorrowerMandatoryField
BorrowerUnwantedField
DefaultPatronSearchFields
UniqueItemFields

Filter to only have Koha's official statues:
decreaseLoanHighHoldsIgnoreStatuses


== prefs with > no ==
They could be ignored now and in a follow up bug, we could share if it's empty/default or custom.

== Maybe a typo in the list ==
ArticleRequestsMandatoryFieldsItemsOnly
↓
ArticleRequestsMandatoryFieldsItemOnly

OpacSuppressionByIPRangeµ
↓
OpacSuppressionByIPRange


== Not in the UI (Administration › System preferences) what does it mean? ==
ElasticsearchIndexStatus_authorities
ElasticsearchIndexStatus_biblios
INTRAdidyoumean
OPACdidyoumean
OpacMainUserBlock
printcirculationslips
opacheader
Comment 18 Victor Grousset/tuxayo 2020-05-06 18:40:43 UTC
@Paul: What was the humorous movement/committee/proposal (I don't remember the nature) of yours that you invented years ago about reducing the number of sysprefs?

Because I'm sign up!
Comment 19 Victor Grousset/tuxayo 2020-05-06 18:41:04 UTC
*signing up
Comment 20 Paul Poulain 2020-05-06 20:44:01 UTC
It was FAUSP : "Fighters Against Useless System Preferences" :D
Comment 21 Katrin Fischer 2020-05-06 21:18:52 UTC
> == Pretty sure they should be > No ==
> Or just share if it's empty/default or custom.
> AdlibrisCoversURL
> EmailAddressForSuggestions
> ILS-DI:AuthorizedIPs
> NewItemsDefaultLocation
> OpacHiddenItems
> OpacHiddenItemsExceptions
> ReplyToDefault
> RestrictedPageLocalIPs
> RestrictedPageTitle
> ReturnpathDefault
> SelfCheckAllowByIPRanges
> UpdateItemLocationOnCheckin

I'd go for not sharing whenever in doubt. I think they would not gain us much, but have the potential to make people insecure about sharing when turning up on Hea maybe?


> == Maybe false positive, could be yes (share) instead of no ==
> OverDrivePasswordRequired
> OverDriveUsername
> NovelistSelectStaffEnabled
> NovelistSelectView
> OAI-PMH:AutoUpdateSets
> OAI-PMH:MaxCount
> opaclanguages
> OpacLocationOnDetail
> OPACResultsLibrary
> RESTOAuth2ClientCredentials
> SyndeticsAuthorNotes
> SyndeticsCoverImages
> SyndeticsCoverImageSize
> SyndeticsEditions
> SyndeticsExcerpt
> SyndeticsReviews
> SyndeticsSeries
> SyndeticsSummary
> SyndeticsTOC

Everything that turns a feature on/off (boolean) is probably ok. Prefs set up with strings might not be very helpful and more dangerous.

opaclanguages would be interesting to see, but would need some extra processing as it's a string of language codes separated by pipe - as a string useless, you'd want to list how often which language is installed.

> == We could just share if it's empty/default or custom ==
> This list will be copied to bug 23898
> AmazonAssocTag
> IntranetCirculationHomeHTML
> IntranetFavicon
> IntranetmainUserblock
> IntranetNav
> IntranetReportsHomeHTML
> IntranetSlipPrinterJS
> intranetstylesheet
> OpacCustomSearch
> OPACHoldsIfAvailableAtPickupExceptions
> OpacLoginInstructions
> OpacMainUserBlock
> OpacMoreSearches
> OPACResultsSidebar
> OPACSearchForTitleIn
> ProcessingFeeNote
> RestrictedPageContent
> RoutingListNote
> SpineLabelFormat
> XSLTDetailsDisplay
> 
> 
> == Not sure, does any know more about these sysprefs? ==
> Can field lists be an issue? They might just be useless actually.
> Or we could just want to know if it's empty/default or custom.
> Precedent: NotesBlacklist is currently marked as > No
> AuthoritySeparator
> BibtexExportAdditionalFields
> ExportRemoveFields
> ISBD
> MarcFieldsToOrder
> MarcItemFieldsToOrder
> MergeReportFields
> OPACISBD
> PatronQuickAddFields
> RisExportAdditionalFields
> StatisticsFields
> SubfieldsToAllowForRestrictedBatchmod
> SubfieldsToAllowForRestrictedEditing
> SubfieldsToUseWhenPrefill
> SubscriptionDuplicateDroppedInput
> 
> Similar doubts as previous paragraph:
> ItemsDeniedRenewal
> LinkerOptions
> MARCAuthorityControlField008
> UNIMARCAuthorityField100
> 
> Are backends lists an issue? Or is it actually very useful?
> ILLOpacbackends

These are not the installed ones, but what works in OPAC, so maybe not.

> Could the directory name be private and thus not sharable?
> intranet_includes
> 
> Could some codes be so rare that they are equivalent of sharing the library
> name?
> MARCOrgCode
> 
> That should be ok right?

The MARCOrgCode is the unique identifier of a library - so it IS equivalent to sharing the library name. Official MARCOrgCodes can be looked up easily.


> borrowerRelationship
> BorrowersTitles
> 
> 
> == We might want to filter the content of these ==
> To only get Koha's official fields and not custom DB fields.
> They could be ignored now and be in a follow up bug.
> Or we could just want to know if it's empty/default or custom.
> BorrowerMandatoryField
> BorrowerUnwantedField
> DefaultPatronSearchFields
> UniqueItemFields
> 
> Filter to only have Koha's official statues:
> decreaseLoanHighHoldsIgnoreStatuses
> 
> 
> == prefs with > no ==
> They could be ignored now and in a follow up bug, we could share if it's
> empty/default or custom.
> 
> == Maybe a typo in the list ==
> ArticleRequestsMandatoryFieldsItemsOnly
> ↓
> ArticleRequestsMandatoryFieldsItemOnly
> 
> OpacSuppressionByIPRangeµ
> ↓
> OpacSuppressionByIPRange

Where is the typo?

> == Not in the UI (Administration › System preferences) what does it mean? ==
> ElasticsearchIndexStatus_authorities
> ElasticsearchIndexStatus_biblios
> INTRAdidyoumean
> OPACdidyoumean

These are set up on their own configuration pages, but systempreferences is used for storage. That's why they are not showing in the regular GUI.

> OpacMainUserBlock
> opacheader

Both of those have been moved into News and should no longer be present, where did you spot them as prefs?

> printcirculationslips

This one has its own bug somewhere I think.
Comment 22 Katrin Fischer 2020-05-06 21:19:37 UTC
Hm, why is seearchengine not turning up in the list, didn't we start the discussion there?
Comment 23 Jonathan Druart 2020-05-07 08:31:49 UTC
Victor, we don't want more sysprefs.

The goal was to double-check, from the whitelist I provided, if they were all valid and relevant: we don't want personal data to be shared.

A second pass could be done for the new prefs we added since I worked on this.

Don't rethink/redo the whole process please.
Comment 24 Victor Grousset/tuxayo 2020-05-07 23:19:42 UTC
(In reply to Paul Poulain from comment #20)
> It was FAUSP : "Fighters Against Useless System Preferences" :D

YES


######


(In reply to Katrin Fischer from comment #21)
> > == Pretty sure they should be > No ==
> > Or just share if it's empty/default or custom.
> > AdlibrisCoversURL
> > EmailAddressForSuggestions
> > ILS-DI:AuthorizedIPs
> > NewItemsDefaultLocation
> > OpacHiddenItems
> > OpacHiddenItemsExceptions
> > ReplyToDefault
> > RestrictedPageLocalIPs
> > RestrictedPageTitle
> > ReturnpathDefault
> > SelfCheckAllowByIPRanges
> > UpdateItemLocationOnCheckin
> 
> I'd go for not sharing whenever in doubt. I think they would not gain us
> much, but have the potential to make people insecure about sharing when
> turning up on Hea maybe?


«have the potential to make people insecure» If there was a way to know what was shared without looking at the code. ^^"
But yes, if it's not useful let's not shared these.


> > == Maybe false positive, could be yes (share) instead of no ==
> > OverDrivePasswordRequired
> > OverDriveUsername
> > NovelistSelectStaffEnabled
> > NovelistSelectView
> > OAI-PMH:AutoUpdateSets
> > OAI-PMH:MaxCount
> > opaclanguages
> > OpacLocationOnDetail
> > OPACResultsLibrary
> > RESTOAuth2ClientCredentials
> > SyndeticsAuthorNotes
> > SyndeticsCoverImages
> > SyndeticsCoverImageSize
> > SyndeticsEditions
> > SyndeticsExcerpt
> > SyndeticsReviews
> > SyndeticsSeries
> > SyndeticsSummary
> > SyndeticsTOC
> 
> Everything that turns a feature on/off (boolean) is probably ok. Prefs set
> up with strings might not be very helpful and more dangerous.

It seems most of these are not useful, especially if there is a main syspref shared that already tells that OverDrive, Novelist, OAI-PMH, Syndetics is in use.
I interpreted "> No" as: "there is/could be private info"
But I should have filtered the above list to not put the obviously not useful ones.

> opaclanguages would be interesting to see, but would need some extra
> processing as it's a string of language codes separated by pipe - as a
> string useless, you'd want to list how often which language is installed.

opaclanguages, yes very interesting! Should the processing be on Koha's side or Hea's side?

So besides opaclanguages (which might not be shared directly) any other prefs from this list worth sharing?


> > Are backends lists an issue? Or is it actually very useful?
> > ILLOpacbackends
> 
> These are not the installed ones, but what works in OPAC, so maybe not.

Not sharing right?


> > Could some codes be so rare that they are equivalent of sharing the library
> > name?
> > MARCOrgCode
> > 
> > That should be ok right?
> 
> The MARCOrgCode is the unique identifier of a library - so it IS equivalent
> to sharing the library name. Official MARCOrgCodes can be looked up easily.

Thanks for the info. I though it was pointing to e.g. the Library Of Congress in the USA, not the library itself. So yeah, not sharable.


> > == Maybe a typo in the list ==
> > ArticleRequestsMandatoryFieldsItemsOnly
> > ↓
> > ArticleRequestsMandatoryFieldsItemOnly
> > 
> > OpacSuppressionByIPRangeµ
> > ↓
> > OpacSuppressionByIPRange
> 
> Where is the typo?

In the file in_DB_but_not_shared


> > == Not in the UI (Administration › System preferences) what does it mean? ==
> > ElasticsearchIndexStatus_authorities
> > ElasticsearchIndexStatus_biblios
> > INTRAdidyoumean
> > OPACdidyoumean
> 
> These are set up on their own configuration pages, but systempreferences is
> used for storage. That's why they are not showing in the regular GUI.

Ok, no worries then.


> > OpacMainUserBlock
> > opacheader
> 
> Both of those have been moved into News and should no longer be present,
> where did you spot them as prefs?

In the file in_DB_but_not_shared. It seems Joubu got them from the DB. So maybe updatedatabase spared them.


> > printcirculationslips
> 
> This one has its own bug somewhere I think.

bug 10014
bug 17845


######


(In reply to Katrin Fischer from comment #22)
> Hm, why is seearchengine not turning up in the list, didn't we start the
> discussion there?


It's not marked as no in the file in_DB_but_not_shared so I assumed it will be shared right?

######


(In reply to Jonathan Druart from comment #23)
> Victor, we don't want more sysprefs.

It was a joke after I went through almost all sysprefs (in_DB_but_not_shared) in the UI ^^" (To double-check what data they could contain)
Even knowing the number of them I though 1.5 hours would be enough, but wrong!


> The goal was to double-check, from the whitelist I provided, if they were
> all valid and relevant

wait, which is the whitelist? I might have gone totally in the wrong direction ﷐[U+1F631]﷑


> we don't want personal data to be shared.

No misundestanding on this. This what I had in mind.


> Don't rethink/redo the whole process please.


So "in_DB_but_not_shared" was the wrong file to check?? ﷐[U+1F631]﷑
Comment 25 Jonathan Druart 2020-05-08 09:55:20 UTC
Victor, yes it's the correct file. I am sorry but those comments are hard to read, you should apply the patches and provide patches on top. IMO that will be more readable.

Basically the ones marked with "> No" are the ones I processed already, so no need to talk more about them, unless you find something wrong (ie. marked as No but should be shared, however I am pretty sure there is no error on them).

Work to be done (on top of the patches/files):
1. Identify the "Not sure" and give your opinion
2. Go through the different new prefs listed and make sure I did not miss a "> No" (ie. I want to share 1 but it should not be. Either because there is a privacy concern or it's useless to share it)
3. Provide a patch on top of it, that will be much more readable that those super long comments :)
4. (optional) Do the same work for the new prefs added since I worked on this (19.06.00.044 - f4254cca37)
5. Remove the "no" (and "not sure") and provide a patch for Hea.
Comment 26 Victor Grousset/tuxayo 2020-05-08 14:49:34 UTC
(In reply to Jonathan Druart from comment #25)
> Victor, yes it's the correct file. I am sorry but those comments are hard to
> read, you should apply the patches and provide patches on top. IMO that will
> be more readable.

Ok, I'll try to integrate the content of my comments via patches. I wasn't confident enough to change the patches (fear of "corrupting" your work)
And I'm still wondering how to deal with the many "categories" that I needed to do in my long comment.

Thought thanks to the responses from Katrin and you, now I don't need that many categories.

> Basically the ones marked with "> No" are the ones I processed already, so
> no need to talk more about them, unless you find something wrong (ie. marked
> as No but should be shared, however I am pretty sure there is no error on
> them).

I'll double check my list of potential false positives. Because I included those not having a privacy risk. Because I'm much less competent about the usefulness.
 
> Work to be done (on top of the patches/files):
> 1. Identify the "Not sure" and give your opinion

ok


> [...]
> 4. (optional) Do the same work for the new prefs added since I worked on
> this (19.06.00.044 - f4254cca37)

This is great, you got the exact version!! :D
That's what I didn't know how to reliably find.

> 5. Remove the "no" (and "not sure") and provide a patch for Hea.

Thanks for the plan!
Comment 27 Aleisha Amohia 2024-06-13 21:21:30 UTC
*** Bug 36210 has been marked as a duplicate of this bug. ***
Comment 28 Arthur Suzuki 2024-11-27 14:31:53 UTC
Created attachment 175003 [details] [review]
Bug 23849: Update the list of syspref sent to HEA
Comment 29 Arthur Suzuki 2024-11-27 14:40:17 UTC
Hi,
Given Jonathan's files and method I've updated C4/UsageStats.pm.
I've also checked for new sysprefs in the installer/data/mysql/mandatory/sysprefs.sql.
Some of the newer sysprefs in the SQL file I didn't add to share because they obviously are not meant to be shared (like OverdriveAuthname or ElectreAPIKey).
I think we also need to somehow keep track of any syspref we don't want to get shared, in order not to add them later by error.
A colleague suggested we make a table in HEA to store the syspref value we don't want to be shared.
Comment 30 Arthur Suzuki 2024-11-27 14:47:18 UTC
also, I've re-ordered everything to alphabetical order.
Comment 31 Jonathan Druart 2024-11-27 15:11:11 UTC
(In reply to Arthur Suzuki from comment #29)
> Hi,
> Given Jonathan's files and method I've updated C4/UsageStats.pm.
> I've also checked for new sysprefs in the
> installer/data/mysql/mandatory/sysprefs.sql.
> Some of the newer sysprefs in the SQL file I didn't add to share because
> they obviously are not meant to be shared (like OverdriveAuthname or
> ElectreAPIKey).
> I think we also need to somehow keep track of any syspref we don't want to
> get shared, in order not to add them later by error.
> A colleague suggested we make a table in HEA to store the syspref value we
> don't want to be shared.

We "simply" should enforce the rule to add new sysprefs to this list at the same time they are added to sysprefs.sql
Comment 32 Aleisha Amohia 2024-11-27 19:32:59 UTC
(In reply to Jonathan Druart from comment #31)

> We "simply" should enforce the rule to add new sysprefs to this list at the
> same time they are added to sysprefs.sql

This is a good place to put that: https://wiki.koha-community.org/wiki/Database_updates#Additional_hints
Comment 33 Arthur Suzuki 2024-12-04 13:27:16 UTC
Created attachment 175178 [details] [review]
Bug 23849: (QA follow-up) remove duplicates on deprecated shared sysprefs
Comment 34 Arthur Suzuki 2024-12-04 13:33:28 UTC
Hello,
I've made a followup patch, some of the sysprefs were either deprecated, or renamed.
So I've changed that in C4/UsageStats.pm.
I've been comparing with a list of Sysprefs which i've exported from a freshly installed ktd (with select * from systempreferences) and compared to what was in C4/UsageStats.pm.

For example (bash commands) : 
# comm -13 <(sort sysprefsList ) <(sort shared )
That would give the list of systempreferences which are shared but not in the DB export.
My last patch reduces this list to none.

# comm -23 <(sort sysprefsList ) <(sort shared )
Would give you the list of "not-shared" systempreferences.
Line count of the previous command says 199 systempreferences not shared atm.