🇲🇩Moldova @andrei.vesterli

Additional patch update

andrei.vesterli → created an issue.

Hey @nick.murza

Looks like a workable solution. Can you please prepare an MR for that?

Hey @japerry

Thx for a quick response. Nope, I don't intend to use dev version. Also, I tested with the dev version before I posted this issue, and the same. Here you can see my working patch. I will just use it before the real issue will be fixed.

andrei.vesterli → created an issue.

Good job!

Also, this is the request response:

and here is the input type that seems to be hidden: and no idea how the server treated it, as a file type or something else...

Hi all

I've tested how this works and it looks like I can upload as many files as I want

Here are 25 only, but I've tried with 50 and could easily upload all the files.

Also, why do we need to share that info text if there are no such real limits? I mean, this is a Drupal "batch" and you can upload 10000000 files without issues.

Regards,
Andrei

Hey @the_g_bomb

It looks like there is no such command in https://docs.alphanodes.com/drush/drush10.html 10.x version for drush. It's better to have "devel" way drush commands. I am not sure when I'll find some time to do that but not now.

Thx for your ideas and findings.

Best,
Andy

Hi guys

Thx for this report. I didn't touch this module for a while. I didn't find big issues for D8 & D9. Of course, it has big issues for D10.

It requires 2 versions for normal support.
All I need is time.

andrei.vesterli → made their first commit to this issue’s fork.

Hi @Yasser Samman

Thx for your support for this module. I've pushed a few new changes and It looks to work just perfectly. I will push a new release asap.

andrei.vesterli → made their first commit to this issue’s fork.

Looks fine!

Hello @apemantus

First of all, thanks for your initiative and help with the project. Great job and effort.
I've left a few comments there. Can you please address them so, I've done a review and that seems to work perfectly!

Kind Regards,
Andrei

andrei.vesterli → created an issue.

Approved and merged. Good job!

Looks good. Will address this in the next version.

Hi @chaitanyadessai

thx for your input. Looks fine, just added a few comments. Can you please address them?

Oh...i see your position/vision. Yeah...makes sense. Let this patch leave here. I need it for sure for our project.
Thx for your input.

The test is failing due to the missing dependency for checkout module from commerce core. I am not sure it's a requirement to add this dependency. No idea what is the design for this solution.

andrei.vesterli → created an issue.

andrei.vesterli → created an issue.

Mergeable

andrei.vesterli → made their first commit to this issue’s fork.

Good to go!

andrei.vesterli → made their first commit to this issue’s fork.

andrei.vesterli → made their first commit to this issue’s fork.

Hi all

None of the patches from this issue seems to apply for 10.2.3

Any reroll for this core version?

ion.eftodii → credited andrei.vesterli → .

andrei.vesterli → created an issue.

Can be closed due to the latest changes in the module

Looking on it!

hi @tguages

Sure, I will try to find some time and get it finished. Leave it for now if you feel you can't complete it. You did a great job. I will also need to test it properly.

Hi @tgauges

Thx for a good finding. You've provided a very good solution so, I love it. Let's complete it. I will then push a new release. I've just left few comments there.

Kind regards
Andrei

andrei.vesterli → created an issue.

Yes. I think we'll need to divide it somehow. For now, it's ok that the address API is mostly compatible but in future we may face issues.

Hi @thenchev

Can you review this one and merge it?

andrei.vesterli → created an issue.

Hi @dinazaur

Add a credit for @nina_jeq, please.

Hi @hemant-gupta

Thx for your contribution! Good job and keep it up! I will leave some comments after the review:

• Check this → guide for more information on how to format the README
• You don't need the file LICENSE.txt
• The file js/timezone.js. It's not ok to declare functions inside the attach: of a Drupal behavior. Move them out of that behavior. Also, I am not sure about this statement today = today.toLocaleString("en-US", {timeZone:timezone, hour12:timeformat}); specially en-US
• The file templates/time-clock.html.twig. There is a missing doc block. Check this → guide and add the doxy to the twig file.
• The file src/Plugin/Block/TimeClockBlock.php. It's recommended to add the type of the property
  

  protected 
   DateFormatterInterface $dateFormatter;

  . Also, Are you sure Constructs a new WorkspaceSwitcherBlock instance.?

• The same file as above. This statement:
  

  if ($dateview) {
    $date = $this->dateFormatter->format(time(), 'custom', $date_format);
  }
  else {
    $date = '';
  }
  

  why not like this:

  $date = '';
  if ($dateview) {
    $date = $this->dateFormatter->format(time(), 'custom', $date_format);
  }
  

  . Besides,

• Same file as above. Please, add the doxy for the method generateTimestamp. No declared params in the description, no return type, no params type, etc.
• Same file as above. Why do you need these options wrapped in t function:
  

  $this->t('12/31/1980'),
  $this->t('31/12/1980'),
  $this->t('1980/12/31'),
  

  . I think there is no reason in that.

• File time_clock.module. A part of the $output variable from the hook time_clock_help has no t() wrap. Check the line 18.

Hi @SuyashArzare

Thx for your contribution! Good job and keep it up! I will leave some comments after the review:

• Check this → guide for more information on how to format the README
• Check this → guide for more information on how to provide a composer.json file in the project
• I see that the service TimestampService has a public method, so, it's a best practice and a recommendation to provide an interface for that class for ex: TimestampServiceInterface and declare the public method getFormattedTimestamp
• Do not use the {@inheritdoc} for the TimestampService service as it's not inherited. Please, provide a relevant description like: "The timestamp service class implementation."
• For the protected properties it's a recommendation to indicate the type: protected DateFormatterInterface $dateFormatter;. Same question for the UserBlock class and the properties inside.
• Update the method getFormattedTimestamp by adding the missing doxy, fox ex: public function getFormattedTimestamp(int $timestamp, string $format, string $timezone): string
• The plugin class UserBlock. Replace the REQUEST_TIME with the \Drupal::time()->getRequestTime() but, use the dependency injection for \Drupal::time(). Check this post for more information.
• The plugin class UserBlock. has the method getCacheMaxAge. Do you need it? And then it returns 10?

Hi @apaderno, @W01F

I am in for any consolidation, collaboration, and help to get something better. What I need to know is "What is the plan?" what exactly do we need to do? I see some things to discuss and put up front.

It's not a problem to create those issues, I need to know the direction. The module I've made is more focused on performance, the other modules are more focused on user experience but there is a lack of performance. I hope I am clear here.

I understand you. What is your idea at the end? Consolidation or not? I have some ideas but not sure about them.

Besides, Permissions Filter → is considered as Obsolete so, we can exclude it.

I think I was confused a bit with the comments above. My mistake, sorry @apagerno.

It might be a good idea regarding consolidation/merging with one of the other projects, but I am not sure if I have time to do that for now. I thought about that and yet, decided to make this small module.

I don't like the approach with D10.2 for the permissions page and the way it searches for some permission by name. I feel the same for the admin/modules page.

Hi @apaderno, @WO1F

This module is about optimizing the permissions page. I am not loading all the data into the page but filtering it by a given criteria. All the solutions you mention are falling into a crash when there are many permissions. Please, try to understand the meaning of this module and do not confuse it with others.

Kind regards
Andrei

Hi @MuskanM

Great module and contribution! Keep it up, man. Here are some things to check/fix:

• Remove all the uuid: lines from the config/install/* folder
• no README, no composer.json. Add them, please, and do that according to the Drupal standards.
• Check indentation and formatting for js/calendar_event_notifications.js. It's hard to read and understand it.
• Don't use anonymous functions statement like this click(function (e) { but like this click((e) => {
• File: src/Form/CalendarEventNotificationsSettingsForm.php. Defining the $form before the customization is better. For instance: $form = parent::buildForm($form, $form_state); after line 56, then, the rest of the implementation. There is a token support logic on the line 96. Then, why don't you check it and save it here submitForm? Put all strings in $this->t() function, please. For example: 'Remainder for your upcoming event',
• File: calendar_event_notifications.module. For example, the function calendar_event_notifications_node_presave, don't use if inside another if and so on in the places where you can add an &&

Kind regards
Andrei

Hi @W01F

Glad that you're interested in this module. It has a purpose and a reason to increase the performance of the page. The user experience might not be the best, but it's FAST. This matters.

Can you add some more details on what you're referring to? What exactly do you want to combine?

Kind regards
Andrei