[D8] AP Newsroom

@Bunty No, you should not. That is just a reminder that the project cannot opt into security coverage for the next 10 days; the review goes on as usual.

As side note, please check the email Git is using, as it's not using the email set in https://www.drupal.org/user/3438175/edit for your account. Since the emails are different, drupal.org isn't able to associate your commits with your account.

Hi Bunty Badgujar,

Please look into below issue :

------ -------------------------------------------------------------------------------------------------------------------------------------------------------------
Line modules/ap_newsroom_clone/src/ApDashboardService.php
------ -------------------------------------------------------------------------------------------------------------------------------------------------------------
60 Property Drupal\ap_newsroom_clone\ApDashboardService::$config has unknown class Drupal\ap_newsroom_clone\ap_newsroombase_configobject as its type.
184 Method Drupal\ap_newsroom_clone\ApDashboardService::prepareNodeForClone() should return Drupal\Core\Entity\EntityInterface but return statement is missing.
199 Instantiated class http\Exception\InvalidArgumentException not found.
199 Throwing object of an unknown class http\Exception\InvalidArgumentException.
234 Variable $node might not be defined.
------ -------------------------------------------------------------------------------------------------------------------------------------------------------------

------ ------------------------------------------------------------------------------------------------
Line modules/ap_newsroom_clone/src/Controller/ApDashboardController.php
------ ------------------------------------------------------------------------------------------------
92 Method Drupal\Core\Form\FormBuilderInterface::getForm() invoked with 2 parameters, 1 required.
------ ------------------------------------------------------------------------------------------------

------ ----------------------------------------------------------
Line modules/ap_newsroom_clone/src/Form/ApFeedsSearchForm.php
------ ----------------------------------------------------------
146 Offset 'data' does not exist on bool|string.
------ ----------------------------------------------------------

Line src/ApNewsroomContent.php
------ --------------------------------------------------------------------------------------------------------------------
124 Return typehint of method Drupal\ap_newsroom\ApNewsroomContent::getContent() has invalid type Drupal\ap_newsroom\stringgetFeedOnDemand.
271 Offset 'data' does not exist on bool|string.
------ -----------------------------------------------------------------------------------------------------------------------------------------

As the documentation says, FormBuilderInterface::getForm() uses more than an argument; the method definition shows a single parameter, but any additional arguments are passed on to the functions called by \Drupal::formBuilder()->getForm(), including the unique form constructor function.

The code used by the module is correct.

    if (empty($url)) {
      $this->messenger->addError($this->t("@error", ["@error" => $this->urlNotCorrectError]));
      return FALSE;
    }

There isn't much that can be translated in "@error" since placeholders cannot be translated. Instead of passing a string containing only a placeholder, you can avoid using $this->t() or expand the string to containing more than the placeholder, such as in the following code.

    if (empty($url)) {
      $this->messenger->addError($this->t("Error while sending the request: @error", ["@error" => $this->urlNotCorrectError]));
      return FALSE;
    }

Otherwise, a string like "@error" will be "@error" in any language.

ap_newsroom_clone.dashboard:
  path: '/admin/content/ap-dashboard'
  defaults:
    title: 'AP newsroom dashboard'
    _controller: '\Drupal\ap_newsroom_clone\Controller\ApDashboardController::listFeeds'
  requirements:
    _permission: 'Access the Content overview page'
  options:
    _admin_route: 'TRUE'

The permission is not defined from the modules, nor is it a Drupal core permission.

    if (isset($decodedJsonFeedData['data']['total_items'])) {
      $total_record = $decodedJsonFeedData['data']['total_items'];
      $this->messenger()->addStatus("$total_record records found.");
    }

To be translatable, the string should use a placeholder.

Thank you for your contribution! I am going to update your account.

These are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

I thank all the dedicated reviewers as well.