πŸ‡ΊπŸ‡ΈUnited States @swirt

Florida
Account created on 25 April 2007, almost 17 years ago
#

Merge Requests

Recent comments

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Thank you for reporting this mortona2k I don't have a site running gin at the moment so I appreciate you raising this.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

I just committed mortona2k's patch and removed language entirely from the node and vocabulary table reports.
It will go out shortly with 1.0.25

Thank you @mortona2k for your work on this and keiserjb for raising it.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

@mortona2k thank you for looking into this. This is a good solution. You are right the counting by language makes no sense with the way translation is happening now in Drupal with them all being the same node.

I'll pull out the whole language column from the report because now it is misleading. I'll get this merged shortly.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

This has been fixed and will go out with 1.0.3

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

This has been released as part of 1.0.2

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Will go out with alpha6

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Will go out with alpha 5

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Crediting the people that initially worked on this error and call to action pattern.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Logging has already been added to the existing moveMenuItem(). It just needs to have similar logic for addMenuItem().

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

This has been completed and will go out with alpha5.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

I accidentally added these fine contributors to the wrong issue. Here is the correct issue ✨ Create method to arrange multiple menu items to match a pattern Active

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Crediting others who gathered and contributed in person to help make the error messages as clean and as helpful as possible.

They were incorrectly added to ✨ Add class to move existing menu items from one part of the menu to another. Active by me, by mistake.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

This has been added and will go out with alpha 5.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

This should go out with alpha 5.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

This has been added. There is now a checkbox on the settings page to disable queueing.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

I tested this on a clean install. The starter file of humans.txt appears in the docroot as it should.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

I am sorry, but it seems like people are each looking at this with a slightly different take and I think it is because the purpose is not clear.

The current purpose says

Well, it would be useful to point folks to both the Drupal issue queue as well as the Maintainers.txt file in a humans.txt file.

Could we maybe change this to a user story or two in order to clarify the goal(s)?

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Tested locally and this all solves the problem cleanly. Nicely done @afinnarn. Thank you.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

swirt β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

This has been merged and will go out with 1.0.5 shortly.

Thank you @timcosgrove for reporting this.
Thank you @gnikolovski for writing a great article that helped identify the problem
Huge thank you to @afinnarn for working on this with me and continually pushing to make this solution the "right solution"

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Those were great suggestions @afinnarn. I moved the property classes into their own directory but not inside the plugin. Sometimes there are discovery things built into Plugins so I wanted to keep that directory clean with only plugins in them
I also switched everything over to call the classes by Use and name.

I could not remove the `value` though. If needed we can handle that with a separate issue.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

It seems like this issue might be able to be closed based on the solution in πŸ› Error: Call to a member function getStorage() on null Needs review

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Note: afinnarn and I met on zoom for a bit where we tossed questions and concerns around. I did not ignore his comment with questions above.

There was much discussion about whether to put all the fetched content into the 'value' property or not. I opted not to for the following reasons:

  1. Given the nature and variety of what is fetched, it did not seem right to return any variety of things with a call to getValue().
  2. It seemed like a potential breaking change to take 'value' that has always been a boolean false and suddenly start putting arrays into it.
  3. We could let Drupal know that "fetched" is the main thing by setting it in mainPropertyName(), so I opted to do that.
πŸ‡ΊπŸ‡ΈUnited States swirt Florida

I was hoping this would be resolved by πŸ› Entity Field Fetch should return a value via JSON:API Active but it was not. Also this may not be a bug in the sense that for any other field, if we added a field to an existing content type, we would not expect or want a value to magically show on the published revision of that field that existed before the new field did.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

It is possible that moving the computed field setting to classes on the property definition will also resolve this bug πŸ› Node save required to get newly added fetched field to show up Active as it may remove the need for the existence of setting them in the call to setValue() https://git.drupalcode.org/project/entity_field_fetch/-/blame/1.0.x/src/...

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Crediting @gnikolovski for his article that showed me what was missing.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Digging at this a bit more I think we don't have to worry about multiple calls to the Fetcher, because the only property that needs the heavy lifting of the Fetcher is the "fetched" property

  $properties['target_type'] = DataDefinition::create('string')...
  $properties['target_id'] = DataDefinition::create('string') ...
  $properties['target_field'] = DataDefinition::create('string')
  $properties['fetched_bundle'] = DataDefinition::create('string')...
  // Each of these just needs a little simple logic to determine the property value. They are currently handled by the Fetcher, but could be moved to their own classes if needed.

  $properties['fetched'] = DataDefinition::create('map') ...
  // This is the one that needs the getEntityData() call from the Fetcher.  It is the thing that does the heavy lifting. 

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

I think I found the solution in this article
https://gorannikolovski.com/blog/add-computed-field-jsonapi-response

In the data properties, there is no class to call for the computed properties
https://git.drupalcode.org/project/entity_field_fetch/-/blob/1.0.x/src/P...

I think the solution will involve wiring up the existing Fetcher class that handles all the heavy lifting to be called from maybe a shell class(es) in these property definitions. The trick will be in making it keep state across the properties so that the fetch does not happen four times for each fetched field, because currently the fetcher builds all the properties in one shot.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

In comparing GraphQL results for a fetch paragraph, the problem becomes more visible:

GraphQL results for a fetched paragraph:

          "fieldCcAboveTopOfPage": {
            "fetched": {
              "fieldWysiwyg": [
                {
                  "value": "<h2>Questions about copay balance</h2>\r\n\r\n<p>For questions about the copay balance of your VA health care bill, call us toll free at the number below. You won't need to pay any copays for X-rays, lab tests, preventative tests, and services like health screenings or immunizations.</p>\r\n",
                  "format": "rich_text",
                  "processed": "<h2>Questions about copay balance</h2>\n\n<p>For questions about the copay balance of your VA health care bill, call us toll free at the number below. You won't need to pay any copays for X-rays, lab tests, preventative tests, and services like health screenings or immunizations.</p>"
                }
              ]
            },
            "fetchedBundle": "wysiwyg",
            "targetField": null,
            "targetId": "109546",
            "targetType": "paragraph",
            "value": false
          }
        },

JSON API

"field_cc_top_of_page_content": false,

I believe the JSON API "false" is coming from the value property, and the other properties are not included. The false is present because it is a computed field, and the rest of the properties are computed.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

I am glad this worked out for you with adding the language to the table. I think it surfaced a problem in your migrated data, but I also see that there is a problem with this report and the way it only looks only for the specified default language.
https://git.drupalcode.org/project/content_model_documentation/-/blame/1...
It should really look through all languages. So I am leaving this open until I can get that reworked.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Adding a note that we will want to make sure the grabbing of the data to put in the jsonapi still respects the established caching of the content as it is controlled now.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

This problem first appeared in 8.x-2.10 β†’ so it seems odd to me that this fix does not look at what introduced this bug in the latest release and does a lot of gymnastics to load an entity that should be there originally, and was there just one version ago. This patch solves the problem but it does so by hiding the root problem.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

If the migrated content has not been touched by human editors you could always roll back the migration, the set the language in the migration and run it again.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

That is interesting and may be the cause of the bad counts, though I would think correct node counts should not be language dependent so there still may be something else at play. I will look into the count part soon.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

If it is not something you want public, you can DM it to me in Drupal Slack.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

If you post your migration yml file here I can take a look and see if I can spot what is going on.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

To me that hints that maybe you have revisions turned on but your migration did not create a new revision and only subsequent saves do.

Using the Views bulk operation module would allow you to bulk save them all, that might slave the problem. My bet is your migration configuration is incomplete and needs to be reworked and then an update migration run.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Hi KeiserJB thank you for reporting this. I will look into it after the new year.

Is there anything special / different about your nodes? Are you using workflow or anything on them? Are you using revisions? If you look at recent log messages are there any notices or errors when looking at the node count report?

I am not seeing counts being off on any of my current installs, but this is certainly worth looking into.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

This has been released in 1.0.24

If you add other content types, feel free to open a new issue. It is easier to track than adding them to this issue.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

I committed your patch @lexfunk and then followed up with a couple minor changes

  1. CS cleanup
  2. moved the access check into the getFieldEditLink() method.
  3. Add the link to the table for sibling fields too.

Thank you for all your work on this Lex. It is a really helpful feature.

This if fixed and will go out with 1.0.24.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

I added two new process plugins to the migration tools section.   I just recently created new documentation pages for each of these plugins in  the contributed process plugins guide.  When those get published/approved, I will remove this Migration Tools section as it no longer matches the approach of this page.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Going out with 2.8

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Released with 8.x-2.8

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

This has been created and will go out with the next release.

/**
 * Compares two values, and if the comparison is true, passes the source thru.
 *
 * @MigrateProcessPlugin(
 * id = "gate_comparator",
 * handle_multiples = TRUE
 * )
 *
 * Example usage: Compare value A to value B. If TRUE use the process pipeline
 * source.  If FALSE, use the 'when_false_value'.
 * @code
 * field_some_text_field:
 *   plugin: gate_comparator
 *   value_a: a string or number to compare or 'source' to use the source.
 *   comparison: = comparison to evaluate [=, <, >, !=, <=, >=]
 *   value_b: the other string or number to compare
 *   when_false_value: A value to use if the comparison is FALSE
 *   source:  The source value to use if the comparison is TRUE
 *
 * @endcode
 *
 * Example usage: Choosing the bigger value between two source fields.
 *
 * @code
 *  field_some_text_field:
 *   plugin: gate_comparator
 *   value_a: cost
 *   comparison: '>'
 *   value_b: average_cost
 *   when_false_value: average_cost
 *   source: cost
 *
 * @endcode
 */

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

swirt β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Also for note, The 'source_operations' key was resolved by this, but I could not find

Warning: Undefined array key "inspections" in Drupal\migration_tools\Operations::process() (line 116 of /var/www/web/modules/contrib/migration_tools/src/Operations.php)

Line 116 or anything near it contained anything about inspections.
nor could I find "inspections" as a key anywhere in the codebase.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Thanks @Mohd Sahzad for the patch and @msielski for reporting it.

I altered the patch a bit to use the more modern null coalesce operator.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

@azinck, thank you so much for your work on this and the contribution. I am sorry for letting this sit so long without responding.

I think your approach is right. I need a good way to test this and at the moment I don't have a good existing migration to try it out with. I am going to need a little time to test this out.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

This has been merged and will go out shortly. Thank you both for your contribution on this.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

This is now fixed and supports tokens for source properties and destination properties (fields). It does not at this time support constants.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Hi Lexfunk this is looking pretty good.
Do we need to wrap the return in getFieldEditLink() with a check for the existence of $type_parameter_key?
It looks like $type_parameter_key is only defined if it is a node or a paragraph due to the switch case. Maybe have in intentionally return NULL if $type_parameter_key is empty, so the return can specify Link|null ( rather than Link | void )

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Thank you for the contribution @lexfunk I will try to get to reviewing this in the next couple days.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

swirt β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

Crediting @Cameron Prince because some of his work led to this approach.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

swirt β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States swirt Florida

This went out with alpha2

Production build https://api.contrib.social 0.61.6-2-g546bc20