console - support additional LDAP Organization fields #2552

cmangeat · 2019-03-19T15:16:39Z

Proposal for #2146

fvanderbiest · 2019-05-07T13:01:15Z

There are conflicts, can you update the PR please ?

…ct_class relates to georchestra/georchestra#2552

jeanpommier · 2019-05-22T08:49:16Z

You're keeping the competency area (list of INSEE IDs) in the groupOfMembers' description field ? Woudn't it be more explicit to use a dedicated field in the georchestraOrg object ?

take into account extra org fields from georchestra/georchestra#2552

fvanderbiest · 2019-05-23T12:12:20Z

You're keeping the competency area (list of INSEE IDs) in the groupOfMembers' description field ? Woudn't it be more explicit to use a dedicated field in the georchestraOrg object ?

Very good point Jean.
We should indeed take advantage of this PR to clean the mess, eventually creating a new schema field.

landryb · 2019-05-23T12:37:25Z

Yes, but once and for all... think of the ones who built code that uses the existing DIT schema :)

fvanderbiest · 2019-05-23T12:41:54Z

Yes, but once and for all... think of the ones who built code that uses the existing DIT schema :)

You're a bit late to the game...
I'm not sure I get your point. Would you like us to not use custom fields ?

jeanpommier · 2019-05-23T12:46:20Z

No, I think Landry says not to move this extent information elsewhere; We actually can keep it on the description field. It is just not very obvious which is which, since we'll have a description field on both objects defining the organization.
Anyway, Id' tend to say that if we move it in a dedicated field in the georchestraOrg object, it should not be so difficult to adjust existing apps to use it, right ?
And yet, will your applications support the pending evolution in the LDAP schema ? If not, it will indeed be a good opportunity to clean up a little bit

landryb · 2019-05-23T13:21:35Z

Its fine to use a custom schema instead of abusing existing object fields, and i dont mind if description content is moved to another attr, as long as it's changing at every release.

adding attributes is fine (like for image/url/etc), its renaming/repurposing attributes that can get hairy.

fvanderbiest · 2019-05-24T07:52:49Z

Anyway, Id' tend to say that if we move it in a dedicated field in the georchestraOrg object, it should not be so difficult to adjust existing apps to use it, right ?

Agreed. To my knowledge, only https://github.com/georchestra/foncier makes use of it atm.

as long as it's changing at every release

OK, I get your point now. Yes, we do not break stable releases (well, we try) ;-)

cmangeat · 2019-06-26T12:14:55Z

@groldan here...

fvanderbiest

LGTM, but we'll change the field to store city codes in a second time.

cmangeat · 2019-07-11T15:47:11Z

816c170 need the "right" latest docker image to be available for tests.

groldan · 2019-07-11T15:50:37Z

@cmangeat please rebase on top of master, the branch 60 commits behind master.
It rebases fine, but when running mvn verify -Pit to run integration tests, get a bunch of failures like this one:

[ERROR] Failures: 
[ERROR]   OrgsIT.createAndGet:62 Status expected:<200> but was:<500>
[ERROR]   OrgsIT.createAndGetWithDescription:95 No value at JSON path "$.description": com.jayway.jsonpath.PathNotFoundException: Expected to find an object with property ['description'] in path $ but found 'java.lang.String'. This is not a json object according to the JsonProvider: 'com.jayway.jsonpath.spi.json.JsonSmartJsonProvider'.

groldan

@cmangeat please rebase on top of master, the branch 60 commits behind master.
It rebases fine, but when running mvn verify -Pit to run integration tests, get a bunch of failures like this one:

[ERROR] Failures: 
[ERROR]   OrgsIT.createAndGet:62 Status expected:<200> but was:<500>
[ERROR]   OrgsIT.createAndGetWithDescription:95 No value at JSON path "$.description": com.jayway.jsonpath.PathNotFoundException: Expected to find an object with property ['description'] in path $ but found 'java.lang.String'. This is not a json object according to the JsonProvider: 'com.jayway.jsonpath.spi.json.JsonSmartJsonProvider'.

Other than that the patch looks pretty straightforward, but can't really run a more in-depth review until the branch is building fine.

groldan · 2019-07-11T14:41:55Z

console/src/it/java/org/georchestra/console/integration/IntegrationTestSupport.java

@@ -119,4 +122,10 @@ public ResultActions perform(RequestBuilder requestBuilder) throws Exception {
    public String testName() {
        return testName.getMethodName();
    }
+
+    public String readRessourceToString(String name) throws URISyntaxException, IOException {


typo: readRessourceToString -> readResourceToString

fixed typo and rebased ... thanks.

…on has to json serailized empty

… desc, or logo

groldan

LGTM. If at all, change the title of the PR so that the merge commit message is more informative.

fvanderbiest · 2019-07-23T08:47:03Z

Tested, looks good !

cmangeat force-pushed the org-fields_2 branch from 00192bb to 1b1756e Compare March 19, 2019 15:23

fvanderbiest requested review from fvanderbiest and severo March 20, 2019 08:25

fvanderbiest added this to the 19.06 milestone Mar 20, 2019

fvanderbiest mentioned this pull request May 7, 2019

console - manage more Orgs fields #2146

Closed

cmangeat force-pushed the org-fields_2 branch from ca2d3f4 to 95a0add Compare May 14, 2019 08:38

cmangeat requested a review from groldan May 14, 2019 08:47

fvanderbiest added a commit to georchestra/georchestra.github.io that referenced this pull request May 14, 2019

Merge pull request #88 from georchestra/doc_form_georchestra_org_obje…

7232035

…ct_class relates to georchestra/georchestra#2552

fvanderbiest removed the request for review from severo May 15, 2019 12:24

fvanderbiest added a commit to georchestra/ckanext-georchestra that referenced this pull request May 22, 2019

Merge pull request #14 from georchestra/GSHDF-91

6555628

take into account extra org fields from georchestra/georchestra#2552

fvanderbiest approved these changes Jul 1, 2019

View reviewed changes

groldan requested changes Jul 11, 2019

View reviewed changes

cmangeat added 7 commits July 12, 2019 11:41

refatcor test

64a74f1

base it test for org

313543b

org may not carry a description, get detail on such an org, descripti…

63b0977

…on has to json serailized empty

no need for status (dead code ?)

2d0b28b

adding description in org ext

13b3532

optimize import

2c90361

org description should be resettable to ''

5865c60

cmangeat and others added 15 commits July 12, 2019 11:41

more pretty

9c86dec

raw integration till front-end for testing purpose, org logo support

18b9a7f

validating org url format at admin side, wanting the protocol to be set

48740c0

update methods name and remove comments

f56310a

when etasblished in props, new account form has to ckeck for org url,…

ebefda6

… desc, or logo

set 'see also' on org targetting ext or detail in pending or not

96c22c8

labels for new fields for org infos at admin side

a539d3e

get rid of org detail

4f668d3

schema adapatations for more org fields

b3ed02f

fix a few codacy warnings

c07f7b8

Add imageinput component

e662e90

Use imageinput in org form

ab74fa2

Add translation keys

3441dbc

oups, have to attach orgExt to org...

077fe02

Display org extra fields in user details page

2d52215

cmangeat force-pushed the org-fields_2 branch from 3953c5e to 2d52215 Compare July 12, 2019 09:42

cmangeat added 2 commits July 12, 2019 11:45

fix typo

78d0d8f

fix tests, indirectly relates issue 2646

9326139

groldan approved these changes Jul 15, 2019

View reviewed changes

groldan changed the title ~~Org fields 2~~ Support additional LDAP Organization fields Jul 18, 2019

groldan merged commit 3199e93 into master Jul 18, 2019

fvanderbiest changed the title ~~Support additional LDAP Organization fields~~ console - support additional LDAP Organization fields Jul 23, 2019

fvanderbiest added this to In progress in Geo2France via automation Jul 23, 2019

fvanderbiest added the feature label Jul 23, 2019

fvanderbiest mentioned this pull request Jul 23, 2019

ldap - area list should be stored in custom field #2638

Open

landryb mentioned this pull request Oct 15, 2019

[console] add an admin-only 'internal notes' field on user & org #2773

Closed

fvanderbiest modified the milestones: 19.09, 19.12 Oct 22, 2019

fvanderbiest deleted the org-fields_2 branch December 3, 2019 08:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

console - support additional LDAP Organization fields #2552

console - support additional LDAP Organization fields #2552

cmangeat commented Mar 19, 2019 •

edited by fvanderbiest

fvanderbiest commented May 7, 2019

jeanpommier commented May 22, 2019 •

edited

fvanderbiest commented May 23, 2019

landryb commented May 23, 2019

fvanderbiest commented May 23, 2019

jeanpommier commented May 23, 2019

landryb commented May 23, 2019

fvanderbiest commented May 24, 2019

cmangeat commented Jun 26, 2019

fvanderbiest left a comment

cmangeat commented Jul 11, 2019

groldan commented Jul 11, 2019

groldan left a comment

groldan Jul 11, 2019

cmangeat Jul 12, 2019 •

edited

groldan left a comment

fvanderbiest commented Jul 23, 2019

console - support additional LDAP Organization fields #2552

console - support additional LDAP Organization fields #2552

Conversation

cmangeat commented Mar 19, 2019 • edited by fvanderbiest

fvanderbiest commented May 7, 2019

jeanpommier commented May 22, 2019 • edited

fvanderbiest commented May 23, 2019

landryb commented May 23, 2019

fvanderbiest commented May 23, 2019

jeanpommier commented May 23, 2019

landryb commented May 23, 2019

fvanderbiest commented May 24, 2019

cmangeat commented Jun 26, 2019

fvanderbiest left a comment

Choose a reason for hiding this comment

cmangeat commented Jul 11, 2019

groldan commented Jul 11, 2019

groldan left a comment

Choose a reason for hiding this comment

groldan Jul 11, 2019

Choose a reason for hiding this comment

cmangeat Jul 12, 2019 • edited

Choose a reason for hiding this comment

groldan left a comment

Choose a reason for hiding this comment

fvanderbiest commented Jul 23, 2019

cmangeat commented Mar 19, 2019 •

edited by fvanderbiest

jeanpommier commented May 22, 2019 •

edited

cmangeat Jul 12, 2019 •

edited