CommentFileSizeAuthor
#10 3198954-10.patch6.74 KBsourabhjain

Issue fork cas_server-3198954

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Umit created an issue. See original summary.

albertski made their first commit to this issue’s fork.

albertski’s picture

Status: Active » Needs review
rob230’s picture

Status: Needs review » Needs work

All you did was add version requirement. You need to also remove all the deprecated code otherwise it is not Drupal 9 compatible.

texas-bronius’s picture

Fyi, install https://www.drupal.org/project/upgrade_status and get a nice report. See below for example via drush :)

$ drush upgrade_status:analyze cas_server
 [notice] Processing /.../web/modules/contrib/cas_server.

================================================================================
CAS Server, --
Scanned on Thu, 07/29/2021 - 21:26

FILE: web/modules/contrib/cas_server/src/Configuration/ConfigHelper.php

STATUS         LINE                           MESSAGE                           
--------------------------------------------------------------------------------
Check manually 241  Calls to function fnmatch should not exist.                 
--------------------------------------------------------------------------------

FILE: web/modules/contrib/cas_server/src/Controller/ProxyController.php

STATUS         LINE                           MESSAGE                           
--------------------------------------------------------------------------------
Ignore         125  Call to deprecated constant REQUEST_TIME: Deprecated in     
                    drupal:8.3.0 and is removed from drupal:10.0.0. Use         
                    Drupal::time()->getRequestTime();                           
--------------------------------------------------------------------------------

FILE: web/modules/contrib/cas_server/src/Controller/ServicesListBuilder.php

STATUS         LINE                           MESSAGE                           
--------------------------------------------------------------------------------
Fix now        32   Call to deprecated method getLabel() of class               
                    Drupal\Core\Entity\EntityListBuilder. Deprecated in         
                    drupal:8.0.0 and is removed from drupal:9.0.0. Use          
                    $entity->label() instead. This method used to escape the    
                    entity label. The render system's autoescape is now relied  
                    upon.                                                       
--------------------------------------------------------------------------------
//
// ... etc for a few screens (!) and then this one, too:
FILE: web/modules/contrib/cas_server/cas_server.info.yml

STATUS         LINE                           MESSAGE                           
--------------------------------------------------------------------------------
Check manually 0    Add core_version_requirement: ^8 || ^9 to designate that the
                    module is compatible with Drupal 9. See                     
                    https://drupal.org/node/3070687.                            
--------------------------------------------------------------------------------

aroseman made their first commit to this issue’s fork.

aroseman’s picture

Status: Needs work » Needs review
texas-bronius’s picture

Status: Needs review » Needs work

@aroseman, did you intend to post a patch with your issue status change? As far as this queue goes, per #5 and #6, work still remains beyond just marking the module as D9 compatible.

sourabhjain’s picture

Status: Needs work » Needs review
StatusFileSize
new6.74 KB

Tried to fix the issue which are mentioned in the #6. Please review.

aroseman’s picture

@texas-bronius,

Relatively new to this process. I addressed deprecated code issues and pushed those changes to the issue fork (commit). I have not created a patch. I will look into that process assuming @sourabhjain's patch doesn't address those issues.

jollysolutions’s picture

Status: Needs review » Needs work

Tests need updateing.

aroseman’s picture

Good call @jollysolutions. I'll address the tests later today.

rwohleb’s picture

Issue summary: View changes
Related issues: +#3139767: Automated Drupal Rector fixes
rwohleb’s picture

... ignoring the confusing automated suggestions from upgrade_status, it just needs to be updated to "Drupal\Core\Entity\Query\QueryFactoryInterface" instead. Still the drupal_set_message() bits of course.
Oof, I'm not used to the new issue fork stuff yet. The existing MR #1 fork handles what I thought was still missing.

rwohleb’s picture

I've added MR3 that fixes references to "drupal_set_message()", "REQUEST_TIME", and "entity.query" service.

bucefal91 made their first commit to this issue’s fork.

bucefal91’s picture

hello guys!

There's also another deprecation that we have been missing until now. In web/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php:

  public function toArray() {
    $properties = [];
    /** @var \Drupal\Core\Config\Entity\ConfigEntityTypeInterface $entity_type */
    $entity_type = $this->getEntityType();

    $id_key = $entity_type->getKey('id');
    $property_names = $entity_type->getPropertiesToExport($this->id());
    if (empty($property_names)) {
      throw new SchemaIncompleteException(sprintf("Entity type '%s' is missing 'config_export' definition in its annotation", get_class($entity_type))); <<<< Now all the config entity types must define 'config_export' property in their annotation.
    }
  ........

On the outside, it produces the following error during drush cim:

   The import failed due to the following reasons:                              
  Unexpected error during import with operation create for cas_server.cas_ser  
  ver_service.anything: Entity type &#039;Drupal\cas_server\Entity\CasServerS  
  ervice&#039; is missing &#039;config_export&#039; definition in its annotat  
  ion                                        

Just defining that annotation property with meaningful data hopefully should be enough. I did confirm drush cim/drush cex continue to work properly on a CAS Server config entity.

dimitriskr made their first commit to this issue’s fork.

rob230’s picture

Status: Needs work » Reviewed & tested by the community

Thank you, I've tested merge request 3. No issues with cim/cex.

j_ten_man’s picture

Status: Reviewed & tested by the community » Fixed
umit’s picture

Many thanks to everyone for the support.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.