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

40398 missing field types #653

Merged
merged 23 commits into from
Feb 5, 2024
Merged

40398 missing field types #653

merged 23 commits into from
Feb 5, 2024

Conversation

dai-eastgate
Copy link
Contributor

related to #634

changed log :
Update missing field types

@github-actions
Copy link

github-actions bot commented Oct 10, 2023

Pull Request Test Coverage Report for Build 7750957043

  • -18 of 94 (80.85%) changed or added relevant lines in 8 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.002%) to 79.536%

Changes Missing Coverage Covered Lines Changed/Added Lines %
plugin/Field/Collection/FieldsCollectionBuilderShort.php 6 7 85.71%
plugin/Field/OutputFields.php 3 4 75.0%
plugin/Model/FormModelBuilder/FormModelBuilderDBForm.php 0 2 0.0%
plugin/Field/Collection/FieldLoaderSupervisorValues.php 48 51 94.12%
plugin/Field/DefaultValue/ModelToOutputConverter/DefaultValueModelToOutputConverter.php 7 11 63.64%
plugin/Field/SearchcriteriaFields.php 4 11 36.36%
Files with Coverage Reduction New Missed Lines %
plugin/Model/FormModelBuilder/FormModelBuilderDBForm.php 1 48.84%
Totals Coverage Status
Change from base Build 7408747638: 0.002%
Covered Lines: 7746
Relevant Lines: 9739

💛 - Coveralls

@fredericalpers fredericalpers added this to the v4.17 milestone Oct 12, 2023
@fredericalpers fredericalpers added the QA Issue or Pull request that is in review label Oct 12, 2023
@fredericalpers fredericalpers linked an issue Oct 12, 2023 that may be closed by this pull request
@andernath
Copy link
Contributor

Happy new year @yeneastgate and @dai-eastgate

Could someone please resolve these conflicts? :)

@dai-eastgate
Copy link
Contributor Author

Happy new year @yeneastgate and @dai-eastgate

Could someone please resolve these conflicts? :)

Happy new year! I will resolve it now :)

andernath
andernath previously approved these changes Jan 4, 2024
@yeneastgate yeneastgate requested a review from andernath January 4, 2024 09:22
Copy link

github-actions bot commented Jan 4, 2024

Steps to install the approved version:

  1. Download onoffice-4.16-13-g00be49f3-please-unpack.zip from https://github.com/onOffice-Web-Org/oo-wp-plugin/actions/runs/7407674928.
  2. Unpack the downloaded file to get another .zip file.
  3. Upload that inner .zip file to WordPress.

@andernath
Copy link
Contributor

andernath commented Jan 5, 2024

@dai-eastgate @yeneastgate

While testing on version 4.16-13-g00be49f3 a fatal error occurs every time I create a new applicant form or a new applicant search form.
I also can not open existing forms of type applicant or applicant search form.

2024-01-05 05_16_57-Window

Fatal error: Uncaught The API returned an error with code 170 and the following message: No read permission for this user The request that caused this error was: 
{ "actionid": "urn:onoffice-de-ns:smart:2.5:smartml:action:read", "resourceid": "", "resourcetype": "user", "parameters": { "data": [ "Vorname", "Nachname", "Name", "Kuerzel" ], "filter": { "Nr": [ { "op": "in", "val": [ 21, 25 ] } ] } } } onOffice\WPlugin\API\ApiClientException in /www/htdocs/w01bee5f/w3pm.onofficeweb.com/htdocs/wp-content/plugins/onoffice-for-wp-websites/plugin/API/APIClientExceptionFactory.php:53 
Stack trace: #0 /www/htdocs/w01bee5f/w3pm.onofficeweb.com/htdocs/wp-content/plugins/onoffice-for-wp-websites/plugin/API/APIClientActionGeneric.php(85): onOffice\WPlugin\API\APIClientExcepti in /www/htdocs/w01bee5f/w3pm.onofficeweb.com/htdocs/wp-content/plugins/onoffice-for-wp-websites/plugin/API/APIClientExceptionFactory.php on line 53

Could you please check this?

@yeneastgate
Copy link
Contributor

@andernath I will check and let you know as soon as possible. Thanks!

@yeneastgate
Copy link
Contributor

yeneastgate commented Jan 5, 2024

@andernath @fredericalpers

  • I found the cause of this bug:
    In the code, I used this API "https://apidoc.onoffice.de/actions/datensatz-lesen/user/".
    with the binding conditions:
    You need read permission to query user data via API. This can be set under “Extras->Einstellungen->Benutzer” / “Extras->Settings->User”. Choose the tab “API user”, then the tab “Rights” with the setting “Benutzerdaten über API auslesen” / “Read out user data by API”.

  • My purpose for using this API:
    I need to get information "short name of the user(Kuerzel)" to display the "list Users" for the "supervisor" field.
    => In my opinion, this above bug is because you have not set the "Read out user data by API" permission on onOffice Enterprise.

  • However, we have "another solution":
    This new solution does not involve setting the "Read out user data by API" permission.
    But when using that solution, we can not display the "short name of the user(Kuerzel)" in the "list Users" of the "supervisor" field, like onOffice Enterprise.

image

  • Please choose a suitable solution:

Solution 1: Is it possible to always install the "Read out user data by API" permission on the onOffice Enterprise?
=> If feasible, the current solution will still work well

Solution 2: I would use the "get users" API (https://apidoc.onoffice.de/actions/informationen-abfragen/get-users/) and not involve setting the "Read out user data by API" permission.
Note: it will not be possible to display the short name of the user(Kuerzel) in the "select users" list of the "supervisor" field.

So which solutions that you choose?

@andernath
Copy link
Contributor

@andernath @fredericalpers

  • I found the cause of this bug:
    In the code, I used this API "https://apidoc.onoffice.de/actions/datensatz-lesen/user/".
    with the binding conditions:
    You need read permission to query user data via API. This can be set under “Extras->Einstellungen->Benutzer” / “Extras->Settings->User”. Choose the tab “API user”, then the tab “Rights” with the setting “Benutzerdaten über API auslesen” / “Read out user data by API”.
  • My purpose for using this API:
    I need to get information "short name of the user(Kuerzel)" to display the "list Users" for the "supervisor" field.
    => In my opinion, this above bug is because you have not set the "Read out user data by API" permission on onOffice Enterprise.
  • However, we have "another solution":
    This new solution does not involve setting the "Read out user data by API" permission.
    But when using that solution, we can not display the "short name of the user(Kuerzel)" in the "list Users" of the "supervisor" field, like onOffice Enterprise.

image

  • Please choose a suitable solution:

Solution 1: Is it possible to always install the "Read out user data by API" permission on the onOffice Enterprise? => If feasible, the current solution will still work well

Solution 2: I would use the "get users" API (https://apidoc.onoffice.de/actions/informationen-abfragen/get-users/) and not involve setting the "Read out user data by API" permission. Note: it will not be possible to display the short name of the user(Kuerzel) in the "select users" list of the "supervisor" field.

So which solutions that you choose?

Thanks for your investigation @yeneastgate !
I will discuss this with @fredericalpers.

@fredericalpers
Copy link
Member

@yeneastgate please implement the suggested solution no. 2, without the necessity for the special user rights and displaying the shortname user(Kuerzel). Thank you :)

@yeneastgate
Copy link
Contributor

@fredericalpers @andernath I have implemented solution 2. Please take a look at my demo video and help me to review this PR. Thanks!

fix_supervisor.mp4

andernath
andernath previously approved these changes Jan 15, 2024
Copy link

Steps to install the approved version:

  1. Download onoffice-4.16-15-g4b8a19cc-please-unpack.zip from https://github.com/onOffice-Web-Org/oo-wp-plugin/actions/runs/7525063132.
  2. Unpack the downloaded file to get another .zip file.
  3. Upload that inner .zip file to WordPress.

@yeneastgate
Copy link
Contributor

@andernath I will check and let you know soon.

@yeneastgate yeneastgate force-pushed the 40398-missing-field-types branch from 16dcf8a to 51669e5 Compare January 16, 2024 07:58
@yeneastgate yeneastgate force-pushed the 40398-missing-field-types branch from 6af9ff2 to 64dc621 Compare January 16, 2024 08:43
@yeneastgate
Copy link
Contributor

@andernath

  1. I see no date fields in forms, neither in frontend nor backend. On my enviroment all date fields are text or number inputs.
    You can see it here:
    https://w3pm.onofficeweb.com/formulare/formular-test/
    I double check if I installed the right version and in the search of estate lists the date and tinyint fields are displayed correctly.

The cause of this bug is that the template has been changed so you must copy the template from template.dist/fields.php to folder "onoffice-personalized" and test again

  1. I noticed if I deactivate "Betreuer/Supervisor" in Search criteria in Enterprise, I still can choose it in applicant and applicantSearch forms. This should be fixed.

Correct, this is a bug. I have fixed it. Below is my video test.

missing_field_case2.mp4

3. If I use the search in estate list for a date field, it is empty after my search results are displayed. It should be filled with date I choosed.

I have covered and fixed this bug in PR "#694".

4. Also if I use the search in estate list for a tinyint field, and I choose "NO" it will reset to "not specified". It should still be "NO" choosen.

Correct, this is a bug. I have fixed it. Below is my video test.

missing_field_case4.mp4

Please review this PR for me. Thank!

@yeneastgate yeneastgate requested a review from andernath January 18, 2024 06:44
@andernath
Copy link
Contributor

The cause of this bug is that the template has been changed so you must copy the template from template.dist/fields.php to folder "onoffice-personalized" and test again

@yeneastgate sorry, my mistake. Thanks for your advice. :)

Also thanks for your fixes, I will review and test it again.

andernath
andernath previously approved these changes Jan 30, 2024
Copy link

Steps to install the approved version:

  1. Download onoffice-4.16-25-g2c55befd-please-unpack.zip from https://github.com/onOffice-Web-Org/oo-wp-plugin/actions/runs/7708204113.
  2. Unpack the downloaded file to get another .zip file.
  3. Upload that inner .zip file to WordPress.

@andernath
Copy link
Contributor

andernath commented Jan 31, 2024

@yeneastgate
I switched to the default template from onOffice plugin. But still most date fields are still not rendered as date inputs. They are still simple text inputs.

For example here you see applicant form (left side) and owner form (right side) with the date field "abdatum" from enterprise.
date_type_in_different_forms_frontend

In Backend the same, date field in applicant (left) is correct, date field in owner (right) is just a simple text field:
date_type_in_different_forms_backend

Current State:

  • Date fields of addresses are not rendererd as date fields in all forms
  • Date fields of estates are not rendererd as date fields in owner and contact form

Desired State:
All date type fields in all forms are rendererd as date fields.

My test environment: https://w3pm.onofficeweb.com/formulare/formular-test/

@yeneastgate
Copy link
Contributor

@andernath
I asked you about this issue before (#634 (comment)), but I did not receive a response.
Yes, we will fix it now

@yeneastgate yeneastgate force-pushed the 40398-missing-field-types branch from 9b5ce3d to 3ce5d15 Compare February 1, 2024 11:16
@yeneastgate
Copy link
Contributor

Current State:

  • Date fields of addresses are not rendererd as date fields in all forms
  • Date fields of estates are not rendererd as date fields in owner and contact form

Desired State: All date type fields in all forms are rendererd as date fields.

@andernath I fixed it. Please watch my video demo and let me know your opinions. Thanks!

date.type.mp4

@andernath
Copy link
Contributor

andernath commented Feb 5, 2024

@andernath I asked you about this issue before (#634 (comment)), but I did not receive a response. Yes, we will fix it now

I'm very sorry @yeneastgate ,
@fredericalpers and I missed your comment :/

The video looks good. I will check code and start testing soon. Thank you!

Copy link

github-actions bot commented Feb 5, 2024

Steps to install the approved version:

  1. Download onoffice-4.16-28-gc04e96e0-please-unpack.zip from https://github.com/onOffice-Web-Org/oo-wp-plugin/actions/runs/7782935084.
  2. Unpack the downloaded file to get another .zip file.
  3. Upload that inner .zip file to WordPress.

@andernath andernath merged commit dcb80e1 into master Feb 5, 2024
6 checks passed
@andernath andernath deleted the 40398-missing-field-types branch February 5, 2024 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA Issue or Pull request that is in review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing field types
4 participants