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

Solution for dealing with a LazyCommand - Bug #16160 #16162

Closed
wants to merge 13 commits into from

Conversation

MarkTro
Copy link
Contributor

@MarkTro MarkTro commented Oct 27, 2023

Resolves the problem that the comand can be a LazyCommand instead of the real DoctrineCommand.

bluvulture and others added 2 commits October 24, 2023 13:01
* Added worfklow for cloning PRs

* Typofix pimcore name

* Changed PR clone to branch clone workflow
@github-actions
Copy link

Review Checklist

  • Target branch (11.0 for bug fixes, others 11.x)
  • Tests (if it's testable code, there should be a test for it - get help)
  • Docs (every functionality needs to be documented, see here)
  • Migration incl. install.sql (e.g. if the database schema changes, ...)
  • Upgrade notes (deprecations, important information, migration hints, ...)
  • Label
  • Milestone

@MarkTro
Copy link
Contributor Author

MarkTro commented Oct 27, 2023

It's a little bit confusing in the description of creating a merge request the base branch for bug fixes is "10.5", which is an older version, and in your checklist above is "11.0".

@kingjia90 kingjia90 changed the base branch from 10.6 to 11.1 October 27, 2023 12:19
@kingjia90
Copy link
Contributor

kingjia90 commented Oct 27, 2023

It's a little bit confusing in the description of creating a merge request the base branch for bug fixes is "10.5", which is an older version, and in your checklist above is "11.0".

The checklist is outdated, it should be 11.1, while 10.5 and 10.6 are not supported anymore, they are EOL.

@MarkTro
Copy link
Contributor Author

MarkTro commented Oct 27, 2023

Ok. So your announcements are more relevant than the mentioning in the system messages. :-)

dvesh3 and others added 6 commits November 1, 2023 09:51
… on calculated values (pimcore#16144)

* Add the possibility to filter on date and number types on calculated values

* fix filters when expressions are used instead of class

* Use addDay function indtead of adding seconds

Co-authored-by: Niklas <niklas.brunberg@cag.se>

* refactor fix filters when expressions are used instead of class

---------

Co-authored-by: Niklas <niklas.brunberg@cag.se>
@DuckThom
Copy link
Contributor

Symfony appears to be doing this in the Application

        if ($command instanceof LazyCommand) {
            $command = $command->getCommand();
        }

Wouldn't is be a cleaner solution to use something like this as well? For example:

    public function add(Command $command): ?Command
    {
        if ($command instanceof LazyCommand) {
            $command = $command->getCommand();
        }

        if ($command instanceof DoctrineCommand) {
            $definition = $command->getDefinition();

I just ran into this error aswell (both on Pimcore 11.0 and 11.1) and adding the instanceof LazyCommand check appears to have solved it for me.

@MarkTro
Copy link
Contributor Author

MarkTro commented Nov 14, 2023

@DuckThom : That is also a nice solution. The difference is that every LazyCommand which passes will be loaded. I don't know if this have a big impact or not. I guess not, because the Command might be load during the request anyway.

@kingjia90
Copy link
Contributor

kingjia90 commented Nov 14, 2023

Tests are likely failing due the branch not being up-to-date
image
Could you please pull and merge 11.1 into it?

yariksheptykin and others added 5 commits November 16, 2023 09:15
* Consume all trasports

* Split consume commands in the example

Co-authored-by: Bernhard Rusch <brusch@users.noreply.github.com>

* Document setting up maintenance workers

* Count like a boss

* 🖊️

Co-authored-by: Jacob Dreesen <jacob@hdreesen.de>

* Polish transport explanations

Co-authored-by: Divesh Pahuja <divesh.pahuja@pimcore.com>

* 🖊️ proofread

Co-authored-by: Manon Cassier <127942915+mcassier31@users.noreply.github.com>

---------

Co-authored-by: Bernhard Rusch <brusch@users.noreply.github.com>
Co-authored-by: Jacob Dreesen <jacob@hdreesen.de>
Co-authored-by: Divesh Pahuja <divesh.pahuja@pimcore.com>
Co-authored-by: Manon Cassier <127942915+mcassier31@users.noreply.github.com>
* Deprecate Data::getAsIntegerCast/getAsFloatCast()

* Fix README

* Change to Pimcore 11.2.0
…mcore#15849)

* Editable Date: New config outputIsoFormat

* Doc format optimized

* Fix wrong doc

* Add upgrade notes

* Change to Pimcore 11.2.0
Copy link

sonarcloud bot commented Nov 17, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@MarkTro
Copy link
Contributor Author

MarkTro commented Nov 17, 2023

@kingjia90

Could you please pull and merge 11.1 into it?

Done.

@jdreesen
Copy link
Contributor

@MarkTro you merged 11.x instead of 11.1, so it now contains many unrelated commits.

@kingjia90
Copy link
Contributor

BTW I've already opened a PR #16257 in replace of this branch

@dvesh3
Copy link
Contributor

dvesh3 commented Nov 17, 2023

Closed in favor of #16257. thank you!

@dvesh3 dvesh3 closed this Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
10 participants