Internationalization - Application Terms of Use #6042

JayanthyChengan · 2019-07-19T19:31:51Z

Related Issues

Internationalization - Terms of Use #5303

coveralls · 2019-07-19T19:41:38Z

Coverage decreased (-0.02%) to 19.472% when pulling 9355402 on JayanthyChengan:5303_TermsofUse into 886da9e on IQSS:develop.

scolapasta · 2019-07-24T17:36:26Z

@JayanthyChengan overall this looks good.

I think we should not have EN for lang for all settings, especially since not all settings are user text, so blank lang, which is default, but then you can add language specific ones if you need. So the logic should be if you don't provide a lang, it returns the default (and most settings would do that); if you do provide a language it looks for that language, but if not available returns the default.

JayanthyChengan · 2019-07-25T18:39:13Z

@scolapasta - Thanks for your review. I would like to add some notes of what I tried.
As a first step, I added new column “lang” and when a new row is added(:ApplicationTermsOfUse , "french content" , "fr" ) , it throws the below exception,

INSERT INTO setting (name, content, lang)
VALUES
(:ApplicationTermsOfUse , "french content" , "fr" );

org.postgresql.util.PSQLException: ERROR: duplicate key value violates unique constraint "setting_pkey"

So, I added “lang” to the primary key (name,lang), and updated to default value “en” as primary key cannot be NULL

https://github.com/IQSS/dataverse/blob/b9bed069c01a5587cc1a75c551ff4352dd9fc755/scripts/database/upgrades/upgrade_v4.15.1_to.sql

setting_pkey PRIMARY KEY (name,lang)

Please add your suggestion.

dataversebot · 2019-08-15T14:08:22Z

Can one of the admins verify this patch?

scolapasta · 2019-08-26T20:11:01Z

Hi @JayanthyChengan OK, I had to do some research, but I have a proposed solution. First, let's add a PK to this table that has no semantic meaning (i.e. just an id). Then we can add unique constraint for name,lang. The issue is that unique constraints don't generally work will null, i.e if you had two rows of ':ApplicationTermsOfUse',null, it would not be treated as unique and we'd have issues.

However, I found a (series of) clever solution(s): using coalesce and/or multiple indices. In our case it should be doable with one:

CREATE UNIQUE INDEX unique_settings
    ON setting
       (name, coalesce(lang, ''));

(coalesce returns the first not-null value, so if null, will return ''. We can do this because that would never be a legitimate value for lang - if it wasn't we could use multiple indices).

While we're at it it probably makes sense to add a constraints to make sure lang is never '':

ALTER TABLE setting
   ADD CONSTRAINT non_empty_lang
            CHECK (lang <>'');

Let me know if this makes sense or you'd like some more details. (here is the link to where I discoverd this idea: https://matjaz.it/tutorial-unique-constraint-on-null-values-in-postgresql/)

JayanthyChengan · 2019-08-27T14:11:09Z

Thanks @scolapasta , I will give it a try.

…ofUse

…n NULL values

JayanthyChengan · 2019-08-29T19:09:20Z

@scolapasta - Thanks Gustavo and I implemented the suggested solution using coalesce, it worked. Committed the code, please review. Thanks

scolapasta · 2019-09-04T19:56:10Z

src/main/java/propertyFiles/Bundle.properties

@@ -2383,6 +2383,7 @@ staticSearchFields.dvObjectType=Type
 staticSearchFields.fileTag=File Tag
 staticSearchFields.fileAccess=Access
 staticSearchFields.publicationStatus=Publication Status
+staticSearchFields.subject_ss=Subject


@JayanthyChengan is this supposed to be part of this PR?

@scolapasta - Missing term related to facet Internationalization. hope it's okay to add with this PR.

scolapasta · 2019-09-04T19:58:08Z

scripts/database/upgrades/upgrade_v4.15.1_to.sql

@@ -0,0 +1,13 @@
+ALTER TABLE ONLY setting DROP CONSTRAINT setting_pkey ;
+
+ALTER TABLE setting ADD COLUMN ID SERIAL PRIMARY KEY;


does this add values for the id for existing settings? (it must, right?)

Also, can you update the file name to reference 4.16 (the latest release?)

@scolapasta - Yes, it adds serial number to all existing rows. I will rename the file and commit it.

committed the changes.

kcondon · 2019-09-09T17:37:33Z

@JayanthyChengan Would you refresh from develop? The app version is still 4.15.1.

Thanks!

…ofUse

JayanthyChengan · 2019-09-09T19:04:42Z

@kcondon - merged the latest develop branch.

kcondon · 2019-09-09T20:28:30Z

@JayanthyChengan thanks. I am seeing a 500 error when I load the app in a browser with a sql error in logs:
Caused by: javax.persistence.PersistenceException: Exception [EclipseLink-4002] (Eclipse Persistence Services - 2.5.2.v20140319-9ad6abd): org.eclipse.persistence.exceptions.DatabaseException
Internal Exception: org.postgresql.util.PSQLException: ERROR: column "id" does not exist
Position: 8
Error Code: 0
Call: SELECT ID, CONTENT, LANG, NAME FROM SETTING WHERE ((NAME = ?) AND (LANG IS NULL))
bind => [1 parameter bound]
Query: ReadAllQuery(name="Setting.findByName" referenceClass=Setting sql="SELECT ID, CONTENT, LANG, NAME FROM SETTING WHERE ((NAME = ?) AND (LANG IS NULL))")

JayanthyChengan · 2019-09-10T14:02:38Z

Hi @kcondon - there is a setting table update, I am not sure whether you executed the db update script? Let me know if the error still exists. Thanks
https://github.com/IQSS/dataverse/blob/f199fce7b3146877d9576bb474fcd3f3186cdfd7/scripts/database/upgrades/upgrade_v4.16_to_4.16.1.sql

kcondon · 2019-09-10T15:44:42Z

@JayanthyChengan We now use an automatic db update management s/w called Flyway. We no longer use the updates dir method. If you would use the Flyway method, that would be great: http://guides.dataverse.org/en/4.16/developers/sql-upgrade-scripts.html# Thanks!

JayanthyChengan · 2019-09-10T18:19:23Z

@kcondon - Added the db upgrade file as per Flyway method. Please verify it now. Thanks

kcondon · 2019-09-10T22:02:13Z

@JayanthyChengan Almost there! The new script needs to be renamed: you need to use a capital V rather a v at the beginning of the script name, otherwise flyway ignores the script. Annoying, I know.

JayanthyChengan · 2019-09-11T13:48:06Z

@kcondon - got it, and renamed the file .

Internationalization - Application Terms of Use

b9bed06

JayanthyChengan mentioned this pull request Jul 19, 2019

Internationalization - Terms of Use #5303

Closed

djbrooke assigned scolapasta Jul 23, 2019

scolapasta assigned JayanthyChengan Jul 24, 2019

JayanthyChengan added 2 commits August 27, 2019 14:15

Merge remote-tracking branch 'remotes/origin/develop' into 5303_Terms…

de60560

…ofUse

Internationalization - Application Terms of Use - UNIQUE constraint o…

222c19a

…n NULL values

pdurbin unassigned JayanthyChengan Aug 29, 2019

scolapasta requested changes Sep 4, 2019

View reviewed changes

renamed the sql file

5b330aa

scolapasta approved these changes Sep 5, 2019

View reviewed changes

scolapasta removed their assignment Sep 5, 2019

added notes to upload language specific "Terms of Use" file

74131c4

pdurbin mentioned this pull request Sep 9, 2019

5303 termsofuse #5304

Closed

kcondon self-assigned this Sep 9, 2019

Merge remote-tracking branch 'remotes/origin/develop' into 5303_Terms…

f199fce

…ofUse

kcondon assigned JayanthyChengan and unassigned kcondon Sep 9, 2019

moved sql upgrade script as per "Flyway" tool documentation

b206c49

landreev mentioned this pull request Sep 10, 2019

5028 Add dataset level external tools #6059

Merged

renamed flyway sql upgrade script

9355402

kcondon assigned kcondon and unassigned JayanthyChengan Sep 11, 2019

kcondon merged commit 61cac63 into IQSS:develop Sep 11, 2019

landreev mentioned this pull request Sep 17, 2019

Settings API broken? #6173

Closed

landreev added a commit that referenced this pull request Sep 17, 2019

fixes the issue with the settings api introduced by #6042. (#6173)

4820d2c

djbrooke added this to the 4.17 milestone Sep 18, 2019

JayanthyChengan mentioned this pull request Sep 24, 2019

Application Terms of Use scholarsportal/dataverse#61

Merged

pdurbin mentioned this pull request Apr 28, 2020

Bug in ApplicationTermsOfUse API #6862

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Internationalization - Application Terms of Use #6042

Internationalization - Application Terms of Use #6042

JayanthyChengan commented Jul 19, 2019

coveralls commented Jul 19, 2019 •

edited

Loading

scolapasta commented Jul 24, 2019

JayanthyChengan commented Jul 25, 2019

dataversebot commented Aug 15, 2019

scolapasta commented Aug 26, 2019 •

edited

Loading

JayanthyChengan commented Aug 27, 2019

JayanthyChengan commented Aug 29, 2019

scolapasta Sep 4, 2019

JayanthyChengan Sep 5, 2019

scolapasta Sep 4, 2019

JayanthyChengan Sep 5, 2019 •

edited

Loading

JayanthyChengan Sep 5, 2019

kcondon commented Sep 9, 2019

JayanthyChengan commented Sep 9, 2019

kcondon commented Sep 9, 2019 •

edited

Loading

JayanthyChengan commented Sep 10, 2019

kcondon commented Sep 10, 2019

JayanthyChengan commented Sep 10, 2019

kcondon commented Sep 10, 2019

JayanthyChengan commented Sep 11, 2019

		@@ -0,0 +1,13 @@
		ALTER TABLE ONLY setting DROP CONSTRAINT setting_pkey ;

		ALTER TABLE setting ADD COLUMN ID SERIAL PRIMARY KEY;

Internationalization - Application Terms of Use #6042

Internationalization - Application Terms of Use #6042

Conversation

JayanthyChengan commented Jul 19, 2019

Related Issues

coveralls commented Jul 19, 2019 • edited Loading

scolapasta commented Jul 24, 2019

JayanthyChengan commented Jul 25, 2019

dataversebot commented Aug 15, 2019

scolapasta commented Aug 26, 2019 • edited Loading

JayanthyChengan commented Aug 27, 2019

JayanthyChengan commented Aug 29, 2019

scolapasta Sep 4, 2019

Choose a reason for hiding this comment

JayanthyChengan Sep 5, 2019

Choose a reason for hiding this comment

scolapasta Sep 4, 2019

Choose a reason for hiding this comment

JayanthyChengan Sep 5, 2019 • edited Loading

Choose a reason for hiding this comment

JayanthyChengan Sep 5, 2019

Choose a reason for hiding this comment

kcondon commented Sep 9, 2019

JayanthyChengan commented Sep 9, 2019

kcondon commented Sep 9, 2019 • edited Loading

JayanthyChengan commented Sep 10, 2019

kcondon commented Sep 10, 2019

JayanthyChengan commented Sep 10, 2019

kcondon commented Sep 10, 2019

JayanthyChengan commented Sep 11, 2019

coveralls commented Jul 19, 2019 •

edited

Loading

scolapasta commented Aug 26, 2019 •

edited

Loading

JayanthyChengan Sep 5, 2019 •

edited

Loading

kcondon commented Sep 9, 2019 •

edited

Loading