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

Update to generator-jhipster v8 #926

Merged
merged 46 commits into from
Oct 20, 2023
Merged

Update to generator-jhipster v8 #926

merged 46 commits into from
Oct 20, 2023

Conversation

mshima
Copy link
Member

@mshima mshima commented Sep 6, 2023

Fixes #927

@mshima mshima marked this pull request as ready for review October 4, 2023 20:50
@DanielFran DanielFran requested a review from mraible October 5, 2023 12:11
@mraible
Copy link
Collaborator

mraible commented Oct 15, 2023

@mshima I tried again and made it to the tutorial's npm run e2e section. Even though this command does start the app on localhost:8100, Cypress cannot reach it for some reason, and all tests fail.

I also noticed that uploading a new image through the web UI used to work. Now it says "Camera plugin not supported" in the console. In the last version, there was an else clause that made it so the web UI worked too.

@mraible
Copy link
Collaborator

mraible commented Oct 15, 2023

There are also a number of warnings in the console about arial-label.

Screenshot 2023-10-15 at 3 36 11 PM

@mshima
Copy link
Member Author

mshima commented Oct 15, 2023

@mshima I tried again and made it to the tutorial's npm run e2e section. Even though this command does start the app on localhost:8100, Cypress cannot reach it for some reason, and all tests fail.

I also noticed that uploading a new image through the web UI used to work. Now it says "Camera plugin not supported" in the console. In the last version, there was an else clause that made it so the web UI worked too.

Try 127.0.0.1:8100 instead.

Edit:
or use (added to workflow).

# https://github.com/cypress-io/cypress/issues/27962#issuecomment-1746294247
NODE_OPTIONS: --dns-result-order=ipv4first

@mshima
Copy link
Member Author

mshima commented Oct 15, 2023

There are also a number of warnings in the console about arial-label.

Screenshot 2023-10-15 at 3 36 11 PM

@mraible there is no change to templates.
Only changes related to jhipster v8 like properties renames.

Maybe it's related to #961. We can try to revert.

@mraible
Copy link
Collaborator

mraible commented Oct 16, 2023

Try 127.0.0.1:8100

Why is this necessary for this blueprint but not for the main generator?

export NODE_OPTIONS=--dns-result-order=ipv4first

This fails, too:

** Angular Live Development Server is listening on localhost:8100, open your browser on http://localhost:8100/ **

✔ Compiled successfully.
[35874:1016/100931.054626:ERROR:node_bindings.cc(300)] Most NODE_OPTIONs are not supported in packaged apps. See documentation for more details.

@mshima
Copy link
Member Author

mshima commented Oct 16, 2023

Try 127.0.0.1:8100

Why is this necessary for this blueprint but not for the main generator?

It’s using 127.0.0.1 jhipster/generator-jhipster@2048035

export NODE_OPTIONS=--dns-result-order=ipv4first

This fails, too:

** Angular Live Development Server is listening on localhost:8100, open your browser on http://localhost:8100/ **

✔ Compiled successfully.
[35874:1016/100931.054626:ERROR:node_bindings.cc(300)] Most NODE_OPTIONs are not supported in packaged apps. See documentation for more details.

It’s probably a warning.
NODE_OPTION in this case is a runtime config. It will not work for compile time.

@mshima
Copy link
Member Author

mshima commented Oct 16, 2023

It’s using 127.0.0.1 jhipster/generator-jhipster@2048035

The main generator uses very few access from outside the container limited to npm scripts.
Ionic on the other hand is outside backend containers, we have many localhost references.
Looks more appropriate to use the NODE_OPTION option.
We can switch to 127.0.0.1.
Unfortunately tutorials should be updated too.
You can try node 20 to see if this behavior persists.

@mraible
Copy link
Collaborator

mraible commented Oct 17, 2023

I tried changing cypress.config.ts to use the IP for the baseUrl:

baseUrl: 'http://127.0.0.1:8100/',

However, the tests still use localhost. So I changed angular.json to have the IP too:

"e2e": {
  "builder": "@cypress/schematic:cypress",
  "options": {
    "devServerTarget": "app:serve",
    "watch": false,
    "headed": true,
    "browser": "chrome",
    "baseUrl": "http://127.0.0.1:8100/"
  },

This makes it so 127.0.0.1 is used, but the tests still fail to resolve the website. I can click on the website when the tests are running, and it loads.

@mraible
Copy link
Collaborator

mraible commented Oct 17, 2023

For some reason, http://127.0.0.1:8100 doesn't work for me in Chrome or Firefox, but http://localhost:8100 does.

@mraible
Copy link
Collaborator

mraible commented Oct 17, 2023

If I run npm start in one terminal and npm run e2e in the other, the tests run successfully. I don't think it used to be this way. The image upload test does fail since it's currently only enabled when the Camera plugin is installed.

@mshima
Copy link
Member Author

mshima commented Oct 17, 2023

@mraible cypress should be fixed with camera capacitor.

@mshima
Copy link
Member Author

mshima commented Oct 17, 2023

I've tested using the following jdl:

application {
  config {
  }
  entities *
}

entity Album {
  title String required
  description TextBlob
  created Instant
}

entity Photo {
  title String required
  description TextBlob
  image ImageBlob required
  height Integer
  width Integer
  taken Instant
  uploaded Instant
}

entity Tag {
  name String required minlength(2)
}

relationship ManyToOne {
  Album{user(login)} to User with builtInEntity
  Photo{album(title)} to Album
}

relationship ManyToMany {
  Photo{tag(name)} to Tag{photo}
}

paginate Album with pagination
paginate Photo, Tag with infinite-scroll

Start backend with ionic cors:

cd backend
jhipster jdl flicker2.jdl
npm run ci:e2e:prepare
./mvnw -Pprod -Dspring.profiles.active=prod,e2e-cors

In another terminal run e2e:

cd ionic4j
npm install
npm run e2e

@mshima
Copy link
Member Author

mshima commented Oct 17, 2023

I didn't had to use 127.0.0.1 or NODE_OPTIONS for this test.

@mraible
Copy link
Collaborator

mraible commented Oct 17, 2023

@mshima I think this change is necessary to fix image upload when going through a browser.

@mshima
Copy link
Member Author

mshima commented Oct 17, 2023

@mshima I think this change is necessary to fix image upload when going through a browser.

Why? It's working for me when I click at album button.

@mshima
Copy link
Member Author

mshima commented Oct 17, 2023

It's possible to force the file dialog by setting webUseInput: true at imageOptions.

@mshima
Copy link
Member Author

mshima commented Oct 17, 2023

This is what appears for me, see the album button at bottom left:
Captura de Tela 2023-10-17 às 19 33 31

Camera works too, I've disabled to test.

Tested using Safari, Chrome and Firefox

@mraible
Copy link
Collaborator

mraible commented Oct 18, 2023

@mshima What OS are you on? On my MacBook Pro M1 with macOS Ventura 13.6, I only see an error in the console:

Camera plugin not supported

@mshima
Copy link
Member Author

mshima commented Oct 18, 2023

MacBook Air M1 with macOS 13.4

@mraible
Copy link
Collaborator

mraible commented Oct 18, 2023

@mshima This doesn't work for me. I tried in Chrome, Firefox, and Safari and the results are the same. Here's a video.

photo-upload.mp4

It's possible Okta is preventing this on my machine. However, I have no other machine to test it on.

Regarding npm run e2e. It doesn't work on the backend app either, so it's possible there are other restrictions that prevent me from using it. When I run the command on the backend, it errors out after the first test.

The automation client disconnected. Cannot continue running tests.

mraible and others added 2 commits October 18, 2023 16:29
* Fix image upload for web UI

* Fix template

* Apply suggestions from code review

* fix references

* add useCameraCapacitor

* use input conditional on !useCameraCapacitor

* Update generators/ionic/templates/src/app/pages/entities/_entity-update.ts.ejs

* Update generators/ionic/templates/src/app/pages/entities/_entity-update.ts.ejs

* Update _entity-update.ts.ejs

* Update generators/ionic/templates/src/app/pages/entities/_entity-update.ts.ejs

---------

Co-authored-by: Marcelo Shima <marceloshima@gmail.com>
@mraible
Copy link
Collaborator

mraible commented Oct 20, 2023

@mshima I can confirm that this branch fixes image upload on the web. I still can't run npm run e2e locally, but I'll approve since the CI tests are passing.

FWIW, it works if I run npm run e2e in a new project from JHipster's main branch. So, it isn't a Chrome issue.

@mshima
Copy link
Member Author

mshima commented Oct 20, 2023

e2e is not prepared for the fallback.
I don't think it's necessary since the non fallback works at docker and probably most CI environments.

@mshima mshima merged commit f9a5ade into jhipster:main Oct 20, 2023
8 checks passed
@mshima mshima deleted the jhipster-v8 branch October 20, 2023 15:19
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.

Migrate to JHipster v8
2 participants