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

Merge branch '2.3' of ezsystems/ezplatform-admin-ui into 4.5 #1221

Merged
merged 2 commits into from
Mar 28, 2024

Conversation

ezrobot
Copy link
Contributor

@ezrobot ezrobot commented Mar 26, 2024

Cross merge PR

@barw4
Copy link
Contributor

barw4 commented Mar 26, 2024

@bogusez I think we should test this on v4.5 just to be on the safe side

@barw4 barw4 requested a review from bogusez March 26, 2024 13:06
@bogusez
Copy link
Contributor

bogusez commented Mar 26, 2024

@bogusez I think we should test this on v4.5 just to be on the safe side

Ok, I'm on it.

@bogusez
Copy link
Contributor

bogusez commented Mar 26, 2024

@barw4
JS exception issue gone but now we have an issue with notification. See more details on attached screenshot

Screenshot 2024-03-26 at 14 35 22

@barw4
Copy link
Contributor

barw4 commented Mar 26, 2024

Thanks, will take a look

@barw4
Copy link
Contributor

barw4 commented Mar 27, 2024

@bogusez did you run the composer run post-install-cmd command after applying the patch?

@bogusez
Copy link
Contributor

bogusez commented Mar 27, 2024

@bogusez did you run the composer run post-install-cmd command after applying the patch?

yes, I did. I will try it again.

@bogusez
Copy link
Contributor

bogusez commented Mar 27, 2024

@barw4 I have reinstalled the environment and I have the same issue. Tested both ways with cloned branch and patched changes (in both cases composer run post-install-cmd run after installing changes).
Tested on v4.5.x-dev

Edit: assets also rebuilt after applying changes. Doesn't help to solve the problem.

@barw4 barw4 force-pushed the temp_2.3_to_4.5 branch from a920960 to 2dde025 Compare March 27, 2024 15:37
@barw4
Copy link
Contributor

barw4 commented Mar 27, 2024

@bogusez can you validate if it works now?

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@bogusez
Copy link
Contributor

bogusez commented Mar 28, 2024

@barw4 now we have a different notification message.

Screenshot 2024-03-28 at 08 20 58

@bogusez
Copy link
Contributor

bogusez commented Mar 28, 2024

@barw4 now we have a different notification message.

Screenshot 2024-03-28 at 08 20 58

I think it is the same notification as before but now with correct translation.

@barw4
Copy link
Contributor

barw4 commented Mar 28, 2024

@bogusez so all is good? The message is correct from my point of view.

@bogusez
Copy link
Contributor

bogusez commented Mar 28, 2024

@bogusez so all is good? The message is correct from my point of view.

The expected result in the task description says:

Expected There should be no error and possibly information about the lack of an image in the content

How should we interpret it?

@barw4
Copy link
Contributor

barw4 commented Mar 28, 2024

? But this is behaving exactly the same as in merged 3.3 you tested?

@bogusez
Copy link
Contributor

bogusez commented Mar 28, 2024

? But this is behaving exactly the same as in merged 3.3 you tested?

Not exactly the same, on 3.3 version there wasn't any notification that chosen asset has no image file attached. I was able to publish the content with selected image content which doesn't contains image file.

@barw4
Copy link
Contributor

barw4 commented Mar 28, 2024

@bogusez please, see the PR's description and title ezsystems/ezplatform-admin-ui#2116

@bogusez
Copy link
Contributor

bogusez commented Mar 28, 2024

@bogusez please, see the PR's description and title ezsystems/ezplatform-admin-ui#2116

so we have discrepancies because expected result in task description says completely different thing or maybe I understood it wrong?

@barw4
Copy link
Contributor

barw4 commented Mar 28, 2024

It says exactly the same thing. Mateusz's description mentioned that there is JS error in the console, he attached a screenshot of it.

There should be no error and possibly information about the lack of an image in the content

The JS error is gone and you have information about lack of an image.

@dew326 dew326 merged commit 44388a7 into 4.5 Mar 28, 2024
15 checks passed
@dew326 dew326 deleted the temp_2.3_to_4.5 branch March 28, 2024 10:15
@bogusez
Copy link
Contributor

bogusez commented Mar 28, 2024

? But this is behaving exactly the same as in merged 3.3 you tested?

@barw4 my bad, the notification on v3.3 also appears. Everything is clear now.

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.

8 participants