8000 Refactor application state mechanism by gevorgmansuryan · Pull Request #7361 · humhub/humhub · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Refactor application state mechanism #7361

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 52 commits into from
Feb 26, 2025
Merged

Refactor application state mechanism #7361

merged 52 commits into from
Feb 26, 2025

Conversation

gevorgmansuryan
Copy link
Contributor

@gevorgmansuryan gevorgmansuryan self-assigned this Dec 19, 2024
@gevorgmansuryan gevorgmansuryan marked this pull request as draft December 19, 2024 22:56
@gevorgmansuryan gevorgmansuryan mentioned this pull request Dec 19, 2024
Closed
luke-
luke- reviewed Dec 22, 2024
View reviewed changes
@gevorgmansuryan gevorgmansuryan marked this pull request as ready for review December 24, 2024 23:30
@gevorgmansuryan
Copy link
Contributor Author

@luke- Seems strange things happened after revert. manually fixed now.

@gevorgmansuryan
Copy link
Contributor Author

@luke- unfortunately tests fixes and installationState changes can not be separated.

REgrading isInstalled: I understand the concern about preferring a simpler syntax. One way to balance simplicity and encapsulation could be to introduce a convenience method in the application class itself, like Yii::$app->isInstalled(), that internally delegates to the InstallationState or method in InstallationState like Yii::$app->InstallationState->isInstalled().
This approach keeps the syntax clean while still benefiting from the advantages of the hasState() implementation.

I personally prefer the direct hasState() implementation as it feels more direct and readable to me. It clearly communicates what is being checked without relying on additional wrapper methods, especially since we don’t interact with applicationState frequently.

@luke-
Copy link
Contributor
luke- commented Jan 7, 2025

@gevorgmansuryan If we stay with the state approach, can we also handle InstallationState::isDatabaseInstalled() via States?

e.g.

  • STATE_NOT_INSTALLED=1 (no config file / db configuration)
  • STATE_DB_CONFIGURED=2 (database connection configured)
  • STATE_DB_INITALIZED=3 (migrations installed)
  • STATE_INSTALLED=4 (installer fully run, admin user)

I'm not sure whether the setState() method makes sense, because actually you can only set the final state, installation completed. Since it is stored in the DB.

I am unsure whether the setState() method makes sense because only the STATE_INSTALLED can be set. Since the saving is done in the DB and the other states are “automatically” detected.

So maybe add method:

  • setStateInstalled()
    instead setState()?

@gevorgmansuryan
Copy link
Contributor Author
gevorgmansuryan commented Jan 7, 2025 •
edited
Loading

@luke - The isDatabaseInstalled method can be moved to ApplicationTrait as it checks the actual database connection, not the state.

The setState method is currently used for setting the STATE_DB_CONFIGURED and STATE_INSTALLED states. If we add a STATE_DB_INITIALIZED state as well, it will also use setState. Therefore, I believe there is no need to introduce a new method solely for setting the STATE_INSTALLED state.

@luke-
Copy link
Contributor
luke- commented Jan 8, 2025

@gevorgmansuryan Hmm, would be good if isDatabaseInstalled is also handled by InstallationState.

Here is an untested draft with my suggestion:

https://github.com/humhub/humhub/pull/7372/files#diff-f74432e238c55079c3e9039b8b5c4a7e1f9b4f5dd76779ca9a749c34b8987c3d

What do you think about it? Do you see any problems with this approach?

luke-
luke- requested changes Feb 22, 2025
View reviewed changes
@gevorgmansuryan gevorgmansuryan 6D40 requested a review from luke- February 22, 2025 20:45
luke-
luke- requested changes Feb 24, 2025
View reviewed changes
@luke- luke- merged commit 1ac3c9a into next Feb 26, 2025
5 checks passed
@luke- luke- deleted the enh/refactor-application-state-mechanism branch February 26, 2025 09:53
@luke-
Copy link
Contributor
luke- commented Feb 26, 2025

@gevorgmansuryan Thansk it's now merged and Module tests are also now green against the next branch.

luke- added a commit that referenced this pull request May 9, 2025
@luke- @marc-farre
@yurabakhtin @gevorgmansuryan
Next -> Dev (#7506)
54f9d56 
* Update common.php

* Fix Session Timeout (#6818)

* Updated Composer.lock

* Fix composer lock

* Prototype: Check module requirements before install/update (#6816)

* Prototype: Check module requirements before install/update

* Prevent serialization of requirements check

* Fixed ModuleId

* Use requirements.php file instead of config.php

* Fix module deletion on update

* Add errors to the log

* Test module with `requirements.php`

---------

Co-authored-by: Marc Farré <contact@marc.fun>
Co-authored-by: Yuriy Bakhtin <yurybakh@gmail.com>

* Update php-test.yml

* Autocommit PHP CS Fixer

* reduce dynamic.php (#7328)

* Reduce dynamic.php content - remove `mailer` from dynamic.php

* Autocommit PHP CS Fixer

* Reduce dynamic.php content - remove `mailer` from dynamic.php

* Reduce dynamic.php content - remove `mailer` from dynamic.php

* Reduce dynamic.php content - remove `cache` from dynamic.php

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Autocommit PHP CS Fixer

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Autocommit PHP CS Fixer

* Reduce dynamic.php content

* Reduce dynamic.php content

* Autocommit PHP CS Fixer

* Reduce dynamic.php content

* Update CHANGELOG.md

---------

Co-authored-by: gevorgmansuryan <gevorgmansuryan@users.noreply.github.com>
Co-authored-by: Lucas Bartholemy <luke-@users.noreply.github.com>

* Reduce dynamic.php content params (#7332)

* Reduce dynamic.php content - remove `mailer` from dynamic.php

* Autocommit PHP CS Fixer

* Reduce dynamic.php content - remove `mailer` from dynamic.php

* Reduce dynamic.php content - remove `mailer` from dynamic.php

* Reduce dynamic.php content - remove `cache` from dynamic.php

* Reduce dynamic.php content

* Reduce dynamic.php content params

* Reduce dynamic.php content params

* Reduce dynamic.php content params

* Reduce dynamic.php content params

* Reduce dynamic.php content

* Reduce dynamic.php content

* Autocommit PHP CS Fixer

* Reduce dynamic.php content

* Autocommit PHP CS Fixer

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Autocommit PHP CS Fixer

* Reduce dynamic.php content

* Reduce dynamic.php content

* Autocommit PHP CS Fixer

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

---------

Co-authored-by: gevorgmansuryan <gevorgmansuryan@users.noreply.github.com>

* settings cleanup (#7338)

* Remove horImageScrollOnMobile option #471

* Remove horImageScrollOnMobile option #471

* Remove horImageScrollOnMobile option #471

* Refactor application state mechanism (#7361)

* Check Codeceptions Tests
#473

* Check Codeceptions Tests
#473

* Check Codeceptions Tests
#473

* Check Codeceptions Tests
#473

* Check Codeceptions Tests
#473

* Check Codeceptions Tests
#473

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Autocommit PHP CS Fixer

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState me
EC59
chanism

* Refactor applicationState mechanism - fixes

* Refactor applicationState mechanism - fixes

* Refactor applicationState mechanism - fixes

* Revert "Refactor applicationState mechanism"

This reverts commit 9ffd324.

* Refactor applicationState mechanism - fixes

* Revert "Refactor applicationState mechanism"

This reverts commit 0694c84.

* Refactor applicationState mechanism - fixes

* Refactor applicationState mechanism - fixes

* Refactor applicationState mechanism - fixes/revert

* Refactoring (#7372)

* Fix Test

* Refactor application state mechanism - fixes

* Revert "Refactoring (#7372)"

This reverts commit ce1b39c

* Refactor application state mechanism - fixes

* Refactor application state mechanism - fix tests

* Refactor application state mechanism - fix tests

* Refactor application state mechanism - fixes

* Refactor application state mechanism - fixes

* Refactor application state mechanism - fixes

* Refactor application state mechanism - fixes

* Refactor application state mechanism - fixes

* Refactor application state mechanism - fixes

* Make SetState private

---------

Co-authored-by: gevorgmansuryan <gevorgmansuryan@users.noreply.github.com>
Co-authored-by: Lucas Bartholemy <luke-@users.noreply.github.com>
Co-authored-by: Lucas Bartholemy <lucas@bartholemy.com>

---------

Co-authored-by: Marc Farré <contact@marc.fun>
Co-authored-by: Yuriy Bakhtin <yurybakh@gmail.com>
Co-authored-by: Gevorg Mansuryan <gevorgmansuryan@gmail.com>
Co-authored-by: gevorgmansuryan <gevorgmansuryan@users.noreply.github.com>
luke- added a commit that referenced this pull request May 14, 2025
@luke- @marc-farre
@yurabakhtin @gevorgmansuryan
Next into Develop (#7520)
c000f26 
* Update common.php

* Fix Session Timeout (#6818)

* Updated Composer.lock

* Fix composer lock

* Prototype: Check module requirements before install/update (#6816)

* Prototype: Check module requirements before install/update

* Prevent serialization of requirements check

* Fixed ModuleId

* Use requirements.php file instead of config.php

* Fix module deletion on update

* Add errors to the log

* Test module with `requirements.php`

---------

Co-authored-by: Marc Farré <contact@marc.fun>
Co-authored-by: Yuriy Bakhtin <yurybakh@gmail.com>

* Update php-test.yml

* Autocommit PHP CS Fixer

* reduce dynamic.php (#7328)

* Reduce dynamic.php content - remove `mailer` from dynamic.php

* Autocommit PHP CS Fixer

* Reduce dynamic.php content - remove `mailer` from dynamic.php

* Reduce dynamic.php content - remove `mailer` from dynamic.php

* Reduce dynamic.php content - remove `cache` from dynamic.php

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Autocommit PHP CS Fixer

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Autocommit PHP CS Fixer

* Reduce dynamic.php content

* Reduce dynamic.php content

* Autocommit PHP CS Fixer

* Reduce dynamic.php content

* Update CHANGELOG.md

---------

Co-authored-by: gevorgmansuryan <gevorgmansuryan@users.noreply.github.com>
Co-authored-by: Lucas Bartholemy <luke-@users.noreply.github.com>

* Reduce dynamic.php content params (#7332)

* Reduce dynamic.php content - remove `mailer` from dynamic.php

* Autocommit PHP CS Fixer

* Reduce dynamic.php content - remove `mailer` from dynamic.php

* Reduce dynamic.php content - remove `mailer` from dynamic.php

* Reduce dynamic.php content - remove `cache` from dynamic.php

* Reduce dynamic.php content

* Reduce dynamic.php content params

* Reduce dynamic.php content params

* Reduce dynamic.php content params

* Reduce dynamic.php content params

* Reduce dynamic.php content

* Reduce dynamic.php content

* Autocommit PHP CS Fixer

* Reduce dynamic.php content

* Autocommit PHP CS Fixer

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

* Autocommit PHP CS Fixer

* Reduce dynamic.php content

* Reduce dynamic.php content

* Autocommit PHP CS Fixer

* Reduce dynamic.php content

* Reduce dynamic.php content

* Reduce dynamic.php content

---------

Co-authored-by: gevorgmansuryan <gevorgmansuryan@users.noreply.github.com>

* settings cleanup (#7338)

* Remove horImageScrollOnMobile option #471

* Remove horImageScrollOnMobile option #471

* Remove horImageScrollOnMobile option #471

* Refactor application state mechanism (#7361)

* Check Codeceptions Tests
#473

* Check Codeceptions Tests
#473

* Check Codeceptions Tests
#473

* Check Codeceptions Tests
#473

* Check Codeceptions Tests
#473

* Check Codeceptions Tests
#473

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Autocommit PHP CS Fixer

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism

* Refactor applicationState mechanism - fixes

* Refactor applicationState mechanism - fixes

* Refactor applicationState mechanism - fixes

* Revert "Refactor applicationState mechanism"

This reverts commit 9ffd324.

* Refactor applicationState mechanism - fixes

* Revert "Refactor applicationState mechanism"

This reverts commit 0694c84.

* Refactor applicationState mechanism - fixes

* Refactor applicationState mechanism - fixes

* Refactor applicationState mechanism - fixes/revert

* Refactoring (#7372)

* Fix Test

* Refactor application state mechanism - fixes

* Revert "Refactoring (#7372)"

This reverts commit ce1b39c

* Refactor application state mechanism - fixes

* Refactor application state mechanism - fix tests

* Refactor application state mechanism - fix tests

* Refactor application state mechanism - fixes

* Refactor application state mechanism - fixes

* Refactor application state mechanism - fixes

* Refactor application state mechanism - fixes

* Refactor application state mechanism - fixes

* Refactor application state mechanism - fixes

* Make SetState private

---------

Co-authored-by: gevorgmansuryan <gevorgmansuryan@users.noreply.github.com>
Co-authored-by: Lucas Bartholemy <luke-@users.noreply.github.com>
Co-authored-by: Lucas Bartholemy <lucas@bartholemy.com>

---------

Co-authored-by: Marc Farré <contact@marc.fun>
Co-authored-by: Yuriy Bakhtin <yurybakh@gmail.com>
Co-authored-by: Gevorg Mansuryan <gevorgmansuryan@gmail.com>
Co-authored-by: gevorgmansuryan <gevorgmansuryan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luke- luke- luke- approved these changes

Assignees

@gevorgmansuryan gevorgmansuryan

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gevorgmansuryan @luke-
0