Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix react microservice user relation request #11830

Merged

Conversation

ruddell
Copy link
Member

@ruddell ruddell commented May 20, 2020

Send the entire user object (including login) instead of just the id field

In Angular, we pass the whole User object "user": {"id": "1", "login": "system", "firstName": "System" .... }

In React we only pass the ID "user":{"id":"1"}, which results in an invalid User since login is a required field. This is because the value returned from the select box only returns the id.

Fixed by finding the user in props.user with the matching id.

Fix #11792

send the entire user object (including login) instead of just the id field

Fix jhipster#11792
@mraible
Copy link
Contributor

mraible commented May 20, 2020

@ruddell I attempted to fix this in #11317. Maybe I missed this part?

@MathieuAA
Copy link
Member

MathieuAA commented May 20, 2020

This fails for a very simple app:

application {
  config {
    baseName testreact
    clientFramework react
  }
  entities Member
}

entity Member {
  firstName String required
  lastName String required
}

relationship OneToOne {
  Member{user(login) required} to User
}

the previous code works though. Seems like there should be two implementations.
I'm double-checking...


edit: this doesn't work for monoliths

@MathieuAA
Copy link
Member

MathieuAA commented May 20, 2020

It won't work for monoliths because of the user id's type: we're comparing strings and numbers :(

@ruddell
Copy link
Member Author

ruddell commented May 20, 2020

@MathieuAA Thanks for catching that.

@mraible Your PR only changed the form when the relationship was not required, but the open issue has a required relationship.

With the latest changes, combos mostly work (there are a lot of them). I used the JDL:

JDL app/entity definitions
application {
  config {
    baseName reactJwt
    clientFramework react
  }
  entities *
}

application {
  config {
    baseName reactOauth
    clientFramework react
    authenticationType oauth2
  }
  entities *
}

entity Member {
  firstName String
  lastName String
}
entity MemberDto {
  firstName String
  lastName String
}
entity MemberReq {
  firstName String
  lastName String
}
entity MemberDtoReq {
  firstName String
  lastName String
}

relationship OneToOne {
  Member{user(login)} to User
  MemberReq{user(login) required} to User

  MemberDto{user(login)} to User
  MemberDtoReq{user(login) required} to User
}

dto MemberDto, MemberDtoReq with mapstruct

Entity Info Works?
User Relation, Oauth2
User Relation, Oauth2, DTO ☑️
User Relation required, Oauth2
User Relation required, Oauth2, DTO
User Relation, JWT
User Relation, JWT, DTO
User Relation required, JWT
User Relation required, JWT, DTO

User Relation, Oauth2, DTO doesn't allow "" as userId (comes from the form) so form submission fails with org.hibernate.TransientPropertyValueException. userId has to be null for it to work. Happens in master, so is unrelated to this PR

@ruddell
Copy link
Member Author

ruddell commented May 20, 2020

CI tests were failing because they use a different relationship name than user (for example, oneToOneUser). Also TestTwoRelationshipsSameEntity.json has two relations to user, but with different names (code), so we would need this line for both. The only way I see to fix that is to loop through the relationships.

Since this is only needed for OAuth2 gateways, I added that as a condition as well.

@pascalgrimaud
Copy link
Member

as there are 2 approvals, let's merge this

@pascalgrimaud pascalgrimaud merged commit 9cf6355 into jhipster:master May 26, 2020
@ruddell ruddell deleted the fix-react-ms-user-entity-selection branch May 26, 2020 17:03
@pascalgrimaud pascalgrimaud added this to the 6.9.1 milestone Jun 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

User details are getting null || On React client
4 participants