Rothamsted-Ecoinformatics / farm_rothamsted

Custom farmOS features for Rothamsted Research.
GNU General Public License v2.0
6 stars 1 forks source link

FarmOS Version update: Support Drupal 10 / farmOS v3 #577

Closed mstenta closed 11 months ago

mstenta commented 1 year ago

Starting this draft PR to update the Rothamsted modules for Drupal 10 / farmOS v3.

This only makes the changes to .info.yml files and composer.json.

There is more to do, according to PHPStan:

$ sudo docker exec -it -u www-data farmos-www-1 vendor/bin/phpstan analyze /opt/drupal/web/sites/all/modules/farm_rothamsted
Note: Using configuration file /opt/drupal/phpstan.neon.
 72/72 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ------------------------------------------------------------ 
  Line   farm_rothamsted.module                                      
 ------ ------------------------------------------------------------ 
  147    Variable $asset in empty() always exists and is not falsy.  
 ------ ------------------------------------------------------------ 

 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 
  Line   modules/farm_rothamsted_experiment/farm_rothamsted_experiment.post_update.php                                                            
 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 
  821    Relying on entity queries to check access by default is deprecated in drupal:9.2.0 and an error will be thrown from drupal:10.0.0. Call  
         \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE or FALSE to specify whether access should be checked.                  
         💡 See https://www.drupal.org/node/3201242                                                                                               
 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 
  Line   modules/farm_rothamsted_experiment/src/Form/ExperimentPlotGeometryForm.php                                                               
 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 
  156    \Drupal calls should be avoided in classes, use dependency injection instead                                                             
  161    \Drupal calls should be avoided in classes, use dependency injection instead                                                             
  263    Relying on entity queries to check access by default is deprecated in drupal:9.2.0 and an error will be thrown from drupal:10.0.0. Call  
         \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE or FALSE to specify whether access should be checked.                  
         💡 See https://www.drupal.org/node/3201242                                                                                               
 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 
  Line   modules/farm_rothamsted_experiment/src/Form/ExperimentVariableForm.php                                                                   
 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 
  455    \Drupal calls should be avoided in classes, use dependency injection instead                                                             
  460    \Drupal calls should be avoided in classes, use dependency injection instead                                                             
  611    Relying on entity queries to check access by default is deprecated in drupal:9.2.0 and an error will be thrown from drupal:10.0.0. Call  
         \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE or FALSE to specify whether access should be checked.                  
         💡 See https://www.drupal.org/node/3201242                                                                                               
 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------------------------------------------------- 
  Line   modules/farm_rothamsted_experiment/src/Menu/RothamstedTaskProvider.php                                                    
 ------ -------------------------------------------------------------------------------------------------------------------------- 
  27     Constructor of class Drupal\farm_rothamsted_experiment\Menu\RothamstedTaskProvider has an unused parameter $entity_type.  
 ------ -------------------------------------------------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------------------- 
  Line   modules/farm_rothamsted_experiment/src/Plugin/views/join/RothamstedExperimentAssetLogs.php  
 ------ -------------------------------------------------------------------------------------------- 
  31     \Drupal calls should be avoided in classes, use dependency injection instead                
 ------ -------------------------------------------------------------------------------------------- 

 ------ --------------------------------------------------------------------------------------- 
  Line   modules/farm_rothamsted_experiment/src/Plugin/views/join/RothamstedExperimentLogs.php  
 ------ --------------------------------------------------------------------------------------- 
  31     \Drupal calls should be avoided in classes, use dependency injection instead           
 ------ --------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------------- 
  Line   modules/farm_rothamsted_experiment/src/Plugin/views/join/RothamstedExperimentPlotLogs.php  
 ------ ------------------------------------------------------------------------------------------- 
  31     \Drupal calls should be avoided in classes, use dependency injection instead               
 ------ ------------------------------------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 
  Line   modules/farm_rothamsted_experiment_research/farm_rothamsted_experiment_research.module                                                   
 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 
  404    Relying on entity queries to check access by default is deprecated in drupal:9.2.0 and an error will be thrown from drupal:10.0.0. Call  
         \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE or FALSE to specify whether access should be checked.                  
         💡 See https://www.drupal.org/node/3201242                                                                                               
 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 

 ------ -------------------------------------------------------------------------------- 
  Line   modules/farm_rothamsted_notification/farm_rothamsted_notification.module        
 ------ -------------------------------------------------------------------------------- 
  120    Variable $changed_fields on left side of ?? always exists and is not nullable.  
 ------ -------------------------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 
  Line   modules/farm_rothamsted_quick/farm_rothamsted_quick.post_update.php                                                                      
 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 
  54     Relying on entity queries to check access by default is deprecated in drupal:9.2.0 and an error will be thrown from drupal:10.0.0. Call  
         \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE or FALSE to specify whether access should be checked.                  
         💡 See https://www.drupal.org/node/3201242                                                                                               
  107    Relying on entity queries to check access by default is deprecated in drupal:9.2.0 and an error will be thrown from drupal:10.0.0. Call  
         \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE or FALSE to specify whether access should be checked.                  
         💡 See https://www.drupal.org/node/3201242                                                                                               
 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 

 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 
  Line   modules/farm_rothamsted_quick/src/Plugin/QuickForm/QuickExperimentFormBase.php                                                           
 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 
  168    \Drupal calls should be avoided in classes, use dependency injection instead                                                             
  339    Relying on entity queries to check access by default is deprecated in drupal:9.2.0 and an error will be thrown from drupal:10.0.0. Call  
         \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE or FALSE to specify whether access should be checked.                  
         💡 See https://www.drupal.org/node/3201242                                                                                               
  343    \Drupal calls should be avoided in classes, use dependency injection instead                                                             
  353    Relying on entity queries to check access by default is deprecated in drupal:9.2.0 and an error will be thrown from drupal:10.0.0. Call  
         \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE or FALSE to specify whether access should be checked.                  
         💡 See https://www.drupal.org/node/3201242                                                                                               
  858    \Drupal calls should be avoided in classes, use dependency injection instead                                                             
  862    \Drupal calls should be avoided in classes, use dependency injection instead                                                             
  863    Call to deprecated method getId() of class Drupal\farm_quick\Plugin\QuickForm\QuickFormBase:                                             
         in farm:2.2.0 and is removed from farm:3.0.0.                                                                                            
         Use QuickFormInterface::getQuickId() instead.                                                                                            
  875    \Drupal calls should be avoided in classes, use dependency injection instead                                                             
  890    \Drupal calls should be avoided in classes, use dependency injection instead                                                             
  917    \Drupal calls should be avoided in classes, use dependency injection instead                                                             
  928    \Drupal calls should be avoided in classes, use dependency injection instead                                                             
  929    Call to deprecated method getId() of class Drupal\farm_quick\Plugin\QuickForm\QuickFormBase:                                             
         in farm:2.2.0 and is removed from farm:3.0.0.                                                                                            
         Use QuickFormInterface::getQuickId() instead.                                                                                            
  932    \Drupal calls should be avoided in classes, use dependency injection instead                                                             
  969    Variable $asset_type in empty() is never defined.                                                                                        
 ------ ----------------------------------------------------------------------------------------------------------------------------------------- 

 [ERROR] Found 30 errors                                                                                                
mstenta commented 1 year ago

Another thing to remember: since Rothamsted doesn't use the farm_role_roles module, we may need to add new scopes for Simple OAuth v6.

aislinnpearson commented 1 year ago

I was thinking about this a bit more last night @mstenta and wondered if we should do it sooner rather than later, so any new development work we are currently doing goes into the new version and saves the creation of new issues?

I am happy to leave this to you and Paul though, and really whatever is most convenient for you both in terms of the general role out. I guess I'm really just repeating what I said yesterday. We are happy to go ahead whenever you are.

mstenta commented 1 year ago

Thanks @aislinnpearson! If all goes well then this should happen pretty seamlessly, and shouldn't interrupt other development. Ideally you won't even notice a difference. :-)

In general, the upgrade from Drupal 9 to Drupal 10 just means a few small tweaks to code, if anything, and it means that the module can work with either farmOS v2 or v3. So it's more of a matter of just "getting it ready" for the upgrade. And that should be minimal effort.

Once that's done, I can roll out the update along with other farmOS instances I host. That will be a gradual process and I have some general work left to do on my end to enable that anyway.

This is all to say: the v3 upgrade should be pretty seamless, much like all of the recent upgrades (eg: farmOS 2.2.0, 2.1.0, etc).

paul121 commented 1 year ago

Some of the rothamsted roles will need to be update to use the new quantity bundle permissions in farmOS v3 as well

aislinnpearson commented 1 year ago

Adding this to the next milestone just so I can keep track of it, but I suspect the release will be done separately to that milestone :)

paul121 commented 11 months ago

After chatting today, I think I'll pull out some commits from this PR to separate "supporting D10/Fv3" and "required changes for D10/Fv3"

aislinnpearson commented 11 months ago

Notes from 28/11/2023: We agreed that this release of FarmOS 3.0 could wait to be deployed to the Rothamsted instances. It's going through it's third round of beta testing now, which will probably find a few bugs. As there is no rush to deploy, we can update when the stable version is ready. Issues from this milestone should be compliant with the change over.

paul121 commented 11 months ago

Separate PR: https://github.com/Rothamsted-Ecoinformatics/farm_rothamsted/pull/592