Skip to content
This repository has been archived by the owner on Oct 4, 2023. It is now read-only.

Crm 5.37.1 #194

Open
wants to merge 51 commits into
base: master
Choose a base branch
from
Open

Crm 5.37.1 #194

wants to merge 51 commits into from

Conversation

akosgarai
Copy link

It seems this extensions is not working well with the latest CiviCRM versions. I tested it with 5.37.1 and it seems broken.
The logs are filled with php errors caused by protected _id variable in php form classes. The Backbone libs are missing, due to the deprecation in the CRM.

Changes in this PR:

  • The generated files (civix, Upgrader, DAO) have been regenerated with civix commands
  • The protected _id issue have been resolved with the removal of the _id variables.
  • Backbone, Marionette, Modelbinder,Underscore js libs have been added to the packages directory (latest versions).
  • Changed resource loaders to use the js libs provided by this extension.
  • Update js code as some of the backbone / marionette related functions were deprecated and removed from the current versions of the libs.

I still found 2 issues.

  • When i save a new booking with a contribution, it fails, the booking is not saved. Without contribution it saves the booking, but when i create the contribution on a previously saved booking, the contribution is not saved.
  • When i edit a booking that has unlimited resource selected, it is missing from the edit pages.

I'm still investigating both. Besides of those, this version seems to be working for me, but I'm not sure. I found this extension a couple of weeks ago and i don't know it well.

The visibility of this variable is public in the parent class. The overwitten visibility caused the following errors:
PHP Fatal error:  Access level to CRM_Admin_Form_Resource:: must be public (as in class CRM_Admin_Form)
It bumps the underscore version from 1.5.0 to 1.13.1
The backbone stack is deprecated in the civicrm. In the 5.37.1 version the libs seems to be missing (404 in browser console), so that the
Booking frontend application is broken.
Versions:
- Backbone: 1.4.0 has been added (developer version). I don't know which version was used in the civi.
- Marionette: 4.1.2 has been added (latest version). I think an older (<3) version was used in the civi.
- Modelbinder: 1.1.1 has been added (latest version). I don't know which version was used in the civi.
It is included from the civi backbone packages directory, so that is could be missing also.
Also use the LONG_NAME variable instead of the hardcoded module name
In the browser console the following error appeared:
Uncaught TypeError: Cannot read property 'Model' of undefined
Solution: If the Backbone is undefined, fallback to the window.Backbone.
The Backbone.Marionette is undefined in the current lib. But Marionette is defined, so it could be used instead.
As the Marionette.Module is deprecated and has been removed from the codebase, the utils.js triggered js error.
The utils.js function has been removed and the add-sub-resource app has been extended with the removed logic.
Console errors are not visible on the screen.
Changes:
- entites.js module elimination. The content has been added to the app.js.
- addInitializer has been removed, the onStart function has to be used instead
- move ResourceTableView from views to app as the modules are not supported
modules are deprecated and has been removed from the codebase
Use CRM.$ instead of $, and CRM._ instead of _
On the page header regions they were loaded later than the template finder function excution.
The missing template caused empty content.
As the app.js is getting too long, some of the objects could be moved to separate files.
It helps me understanding the current issues.
As both modal trigger events the handler supposed to be the parent class
To be honest i don't know that it works well or not. Couldn't catch the render dialog event
It is handled with a function in common.js that is already loaded on the form
The current upgrader processes the xml files that has this suffix
civix generate:entity-boilerplate
to resolve Unknown db error during extension install
@mlutfy
Copy link

mlutfy commented Sep 26, 2023

These changes should now be in: https://lab.civicrm.org/extensions/booking/-/tree/main (from merging the mjw branch, I did a few spot checks and looks OK)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants