FAIDARE issueshttps://forgemia.inra.fr/urgi-is/faidare/-/issues2023-09-27T17:18:41+02:00https://forgemia.inra.fr/urgi-is/faidare/-/issues/27Follow-up from "Add creation of a log with last indexing informations. GNP-6338"2023-09-27T17:18:41+02:00Raphaël FloresFollow-up from "Add creation of a log with last indexing informations. GNP-6338"The following discussions from !132 should be addressed:
- [ ] @cyril.pommier started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/132#note_161029): (+2 comments)
> Have you noticed that beta deployed i...The following discussions from !132 should be addressed:
- [ ] @cyril.pommier started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/132#note_161029): (+2 comments)
> Have you noticed that beta deployed is trigger despite the test failure @raphael.flores ?
- [ ] @raphael.flores started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/132#note_175831):
> Need to enable that again if relevant.Test milestone several projectsRaphaël FloresRaphaël Floreshttps://forgemia.inra.fr/urgi-is/faidare/-/issues/26Follow-up from "Draft: Epic/merge faidare dd"2022-09-21T09:49:54+02:00Cyril PommierFollow-up from "Draft: Epic/merge faidare dd"The following discussion from !82 should be addressed:
- [ ] @raphael.flores started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/82#note_95824): (+4 comments)
> No test at all is checking this ValueObj...The following discussion from !82 should be addressed:
- [ ] @raphael.flores started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/82#note_95824): (+4 comments)
> No test at all is checking this ValueObject at this time?Cyril PommierCyril Pommierhttps://forgemia.inra.fr/urgi-is/faidare/-/issues/25Follow-up from "Fix Export germplasms"2022-09-02T16:37:17+02:00Cyril PommierFollow-up from "Fix Export germplasms"The following discussion from !130 should be addressed:
- [ ] @cyril.pommier started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/130#note_107798):
> @celia.michotey , I don't remember how those field g...The following discussion from !130 should be addressed:
- [ ] @cyril.pommier started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/130#note_107798):
> @celia.michotey , I don't remember how those field got supprossed from the export. Does that ring any bell ?
- [ ] @raphael.flores started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/130#note_107894):
> TODO fix.Célia MichoteyCélia Michoteyhttps://forgemia.inra.fr/urgi-is/faidare/-/issues/24Proposal: implement group access verification in Data Discovery2021-10-01T18:09:17+02:00Jean-Baptiste NizetProposal: implement group access verification in Data DiscoveryIn the "old" Faidare application, which was in charge of the searches and of the detailed cards, every access to elastic search targeted one or more indices based on the group IDs of the currently authenticated user.
For reference, thes...In the "old" Faidare application, which was in charge of the searches and of the detailed cards, every access to elastic search targeted one or more indices based on the group IDs of the currently authenticated user.
For reference, these group IDs are obtained by
- looking at the user name in the basic authorization header (in the `AuthenticationFilter`)
- storing it in the `AuthenticationStore` (in a ThreadLocal)
- fetching the group IDs from an in-memory cache, that loads them from an external web service (in `UserGroupsResourceClient`)
Faidare is now split in two separate applications:
- the Faidare flavor of data-discovery, using Spring Data ES
- the Faidare application, still using the same code as the "old" Faidare
Using the same strategy in data-discovery doesn't seem easily doable, because Spring Data considers (and is right to do so, IMHO) that a document belongs to a single index.
So my proposal is
- to keep the Faidare code as is, because changing it is too much a burden for Ninja Squad, and out of our comfort zone anyway.
- to implement access control in a different, simpler way in data-discovery
In details, here's how it would work in data-discovery:
1. The faidare documents would have an additional property of type `int`, named `groupId`. This would have to be populated, as all the other fields, by the ETL
2. In a similar way as in Faidare, the faidare flavor of data-discovery would get the user name from the authorization header, get the user groups for that user, and store them in a cache
3. The FaidareDaoImpl of data-discovery would add a criterion to the queries used for search and aggregation which, in SQL, would be `AND faidare_document.group_id in (:accessibleGroupsOfCurrentUser)`
Note that `accessibleGroupsOfCurrentUser` would always contain the ID `0`, even if the user is not authenticated: 0 is the public group ID.
Please tell if this proposa suits you.Cyril PommierCyril Pommierhttps://forgemia.inra.fr/urgi-is/faidare/-/issues/23Review export2021-09-29T09:41:57+02:00Jean-Baptiste NizetReview exportSee https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/104#note_64782See https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/104#note_64782Célia MichoteyCélia Michoteyhttps://forgemia.inra.fr/urgi-is/faidare/-/issues/21Follow-up from "WIP: Add mcpd developments. Fix GNP-5428."2021-08-20T09:48:59+02:00Célia MichoteyFollow-up from "WIP: Add mcpd developments. Fix GNP-5428."The following discussions from !53 should be addressed:
- [ ] @cyril.pommier started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/53#note_45203):
> That's just a workaround, it would be needed to ration...The following discussions from !53 should be addressed:
- [ ] @cyril.pommier started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/53#note_45203):
> That's just a workaround, it would be needed to rationalize the index naming. If it merges like that, which might be better, we need to create a task for the next releaze to make things cleaner. What do you think @celia.michotey @raphael.flores
- [ ] @cyril.pommier started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/53#note_45207):
> Add many other test to chack that most fields a returned with the right values. Possibly add a deep equals test between germplasm0.json and the returned value.Cyril PommierCyril Pommierhttps://forgemia.inra.fr/urgi-is/faidare/-/issues/20Follow-up from "Feat/implement germplasm result page"2020-08-13T16:18:40+02:00Jérémy DestinFollow-up from "Feat/implement germplasm result page"The following discussions from !46 should be addressed:
- [ ] @cyril.pommier started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/46#note_18821): (+1 comment)
> The name of this class seems odd to me. S...The following discussions from !46 should be addressed:
- [ ] @cyril.pommier started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/46#note_18821): (+1 comment)
> The name of this class seems odd to me. Search criterias should be rather generic, not a search criteria by method. I assume that there is no GermplasmGETSearchCriteria ?
> What does it add on top of GermplasmPOSTSearchCriteria ?
> FaidareGermplasmSearchCriteria is strange too. DetailGermplasmSearchCriteria instead ?
- [ ] @cyril.pommier started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/46#note_18825):
> Redundant with the new method, therefore deprecated no ?
- [ ] @cyril.pommier started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/46#note_18826):
> Redundant with the new method rigth below that one no ? Shouldn't it be deprecated / removed ?
- [ ] @cyril.pommier started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/46#note_18849): (+1 comment)
> comment to remove
- [ ] @cyril.pommier started a [discussion](https://forgemia.inra.fr/urgi-is/faidare/-/merge_requests/46#note_18850):
> killJérémy DestinJérémy Destinhttps://forgemia.inra.fr/urgi-is/faidare/-/issues/16Responsive: small screen fixes2019-06-06T14:15:02+02:00Guillaume CornutResponsive: small screen fixes- Center accession image horizontally on small screen
=> Add class `justify-content-center` on row https://forgemia.inra.fr/urgi-is/gpds/blob/master/frontend/src/app/germplasm-card/germplasm-card.component.html#L159
- Add non breakable...- Center accession image horizontally on small screen
=> Add class `justify-content-center` on row https://forgemia.inra.fr/urgi-is/gpds/blob/master/frontend/src/app/germplasm-card/germplasm-card.component.html#L159
- Add non breakable space ` ` in result section title (otherwise the`:` wraps to a new line on small screens)
=> `s/Results :/Results :/` https://forgemia.inra.fr/urgi-is/gpds/blob/master/frontend/src/app/result-page/result-page.component.html#L50
- Add space to break text in `Acquisition/Creation date` (otherwise the text overflows in small screens)
=> `s|Acquisition/Creation date|Acquisition / Creation date|` https://forgemia.inra.fr/urgi-is/gpds/blob/master/frontend/src/app/germplasm-card/germplasm-card.component.html#L403
- Fix accession image popover placement on small screen. As-is, it overflows to the right of the screen with no possibility to scroll
- Re-layout the trait widget for small screens (might be difficult!)
Example of accession with an image for the above issues:
http://urgi.versailles.inra.fr/gpds/germplasm?id=ZG9pOjEwLjE1NDU0LzEuNDkyMTc4NTc1ODc5Mzg1M0UxMg%3D%3D
ping @celia.michotey @jeremy.destin @Melanie.Buy @guillaume.cornut https://forgemia.inra.fr/urgi-is/faidare/-/issues/15Improve code GPDS2019-06-11T19:49:47+02:00Jérémy DestinImprove code GPDS* [x] Factorize popover templates for institutes (use parameters ?).
* [x] Use `width: 100%` instead of `max-width: 100%` on `.popover` ? (check if it work correctly everywhere)
* [ ] Simplify code https://forgemia.inra.fr/urgi-is/gpds...* [x] Factorize popover templates for institutes (use parameters ?).
* [x] Use `width: 100%` instead of `max-width: 100%` on `.popover` ? (check if it work correctly everywhere)
* [ ] Simplify code https://forgemia.inra.fr/urgi-is/gpds/merge_requests/14#note_7655.
* [x] Pipe to format from number to string coordinate
* [x] Factorize style card (for link)?
* [x] @guillaume.cornut Add tests for `card-section`, `card-row` and `card-table` that can also be used as documentation (all possible ways to use them)
* [x] Extract photos and logos URL in indexes
* [x] Remove scroll bar in xref table (exemple: https://urgi.versailles.inra.fr/gpds/germplasm?id=ZG9pOjEwLjE1NDU0LzEuNDkyMTc4NjkxMzU3MDE3OEUxMg%3D%3D)
* [x] Add full source name in navbar (exemple: CIRAD TropGENE instead of CIRAD)
* [x] Resize map on cards (to be able to see a little more the identification part).
* [ ] If possible, add an option to show a bigger map.
* [x] Rename "Data source" depending on the current card and set full source name ("Link to this [thematic] in [source]")
* [x] Improve display of external link on result page (change the text and/or pur it beside the object name ?)
* [x] Try to see why IBET and VIB logo are not displayed on cards
* [x] Renamer "Collecting" section in "Collector", "Holding" section in "Depositary"
* [x] Move "Origin site" at the end of the "Identification" section
* [x] Replace link below the photo by its copyright with a link style
* [x] On result page, replace `Results :` by `Results:`
* [x] For emails, replace "@" by " at "
* [x] For panel names, replace all "_" by " "
* [x] Add an "_blank" in xrefs links as it should be opened in a new window (external links)19.1Jérémy DestinJérémy Destinhttps://forgemia.inra.fr/urgi-is/faidare/-/issues/8Document deprecated BrAPI fields2019-03-01T14:23:59+01:00Raphaël FloresDocument deprecated BrAPI fields- [ ] For BrAPI field which are deprecated (ie. `backend/src/main/java/fr/inra/urgi/gpds/domain/brapi/v1/data/BrapiLocation.java#getName()` we should add a Javadoc explaining what to do with those fields, ie.:
```java
/**
* Depr...- [ ] For BrAPI field which are deprecated (ie. `backend/src/main/java/fr/inra/urgi/gpds/domain/brapi/v1/data/BrapiLocation.java#getName()` we should add a Javadoc explaining what to do with those fields, ie.:
```java
/**
* Deprecated in favor of {@link #getLocationName()}
*/
@Deprecated
@JsonView(JSONView.BrapiFields.class)
String getName();
```
- [ ] Update code where those deprecated are currently used, ie `backend/src/main/java/fr/inra/urgi/gpds/domain/data/LocationVO.java:150`
- [ ] Update accordingly BrAPI code (ie. VO?) in frontend classes if they still use deprecated code.19.1Jérémy DestinJérémy Destinhttps://forgemia.inra.fr/urgi-is/faidare/-/issues/6Add error reporting from frontend to backend2019-02-11T10:56:46+01:00Guillaume CornutAdd error reporting from frontend to backendThe following discussion from !5 should be addressed:
- [ ] @raphael.flores started a [discussion](https://forgemia.inra.fr/urgi-is/gpds/merge_requests/5#note_5715):
> Later, I think that logging the error at server side could be ...The following discussion from !5 should be addressed:
- [ ] @raphael.flores started a [discussion](https://forgemia.inra.fr/urgi-is/gpds/merge_requests/5#note_5715):
> Later, I think that logging the error at server side could be a good point in order to be able to track real life bugs. For instance with the help of [ngx-logger](https://www.npmjs.com/package/ngx-logger) (or other, but further tech watch needed).Laterhttps://forgemia.inra.fr/urgi-is/faidare/-/issues/5Uniformize cards (common components, common styles, etc.), after merge correc...2019-04-11T15:08:54+02:00Guillaume CornutUniformize cards (common components, common styles, etc.), after merge correctionsFollow up after the site !6, study !7 and germplasm !4 card are merged all together.
@guillaume.cornut @erik.kimmel @jeremy.destin @Melanie.Buy @raphael.flores @celia.michotey
TODO:
- [x] Refactor map component to work with both site...Follow up after the site !6, study !7 and germplasm !4 card are merged all together.
@guillaume.cornut @erik.kimmel @jeremy.destin @Melanie.Buy @raphael.flores @celia.michotey
TODO:
- [x] Refactor map component to work with both site and study card
- [x] Uniformize card SCSS/HTML
- [x] De-duplicate TypeScript models (for BrAPI objects, site, study, germplasm, etc.)
- [x] Reduce cyclomatic complexity? https://forgemia.inra.fr/urgi-is/gpds/commit/073aece96cd6df60415bcd41be1084794b824dd1#note_5567
- [x] study card
- [x] site card
- [x] germplasm card
- [x] Delete unecessary JS dependencies (is jquery and popper really useful?)
- [x] Re-use the same loading pinner on all cards + result page ([use font awesome spinner](https://fontawesome.com/icons/spinner?style=solid)) https://forgemia.inra.fr/urgi-is/gpds/merge_requests/12/diffs
- [x] Remove error handling in site card in favor of the error interceptor service
- [x] @guillaume.cornut Add tests for form tab switching + test KeyVAlueObject
- [ ] Add documenting tests on all usages of card-row, card-table, card-section
- [x] Resolve warnings on `npm install` https://forgemia.inra.fr/urgi-is/gpds/merge_requests/11/diffs
- [x] Error report with some context (back to last page, hide spinner) https://forgemia.inra.fr/urgi-is/gpds/merge_requests/12/diffs
- [x] @guillaume.cornut Code analysis front/back IntelliJ (fix depreciation warning in tests) https://forgemia.inra.fr/urgi-is/gpds/merge_requests/12/diffs
The following discussion from !6 should be addressed:
- [x] @raphael.flores started a [discussion](https://forgemia.inra.fr/urgi-is/gpds/merge_requests/6#note_5427): (+1 comment) => Resolved with addition of card-section/card-row/card-table components
> Don't know the best practice:
> - let the code as is?
> - extract styles in the css file?
>
> Other opinion?
- [x] @raphael.flores started a [discussion](https://forgemia.inra.fr/urgi-is/gpds/merge_requests/6#note_5426): (+1 comment)
> Is it a good practice to construct HTML code into an Angular component?
>
> Couldn't we have a dedicated `IconText` component with its own tests and styles in order to avoid such mix?
- [x] @raphael.flores started a [discussion](https://forgemia.inra.fr/urgi-is/gpds/merge_requests/6#note_5425): (+3 comments)
> What if the given `sites` array is empty?
The following discussion from !5 should be addressed:
- [ ] @raphael.flores started a [discussion](https://forgemia.inra.fr/urgi-is/gpds/merge_requests/5#note_5862): (+2 comments)
> Another thing, the test here does not check the presence of the two tabs, shouldn't we check that the tabs are loaded correctly with the ontology widget also?18.3Jérémy DestinJérémy Destin