Problem/Motivation

On comments #87 and 106-110 of #2920309: Add experimental module for Help Topics, a suggestion was made to refactor this module into a plugin system.

From @Berdir:

So if help pages were plugins, then we could have a config entity too that would expose itself as a plugin. That we, we could have both user-managed help pages as well as module-provided help which would be easy to keep up to date as modules are updated, which I think the current config system can't really do without contrib modules.

From @jhedstrom:

This approach seems reasonable, as all of the work already done here around config entities will still work (a deriver will be needed to loop through them and expose them as HelpTopic plugins). Modules can also hard-code HelpTopic plugins that will live with the code, and be able to be updated for sites as the module evolves.

Next steps to implement that approach from the current patch would be:

  • Define a HelpTopic plugin annotation
  • Create a plugin deriver that loops through all the config-based help topics
  • Instead of looping through the config entities in our HelpSection plugin, loop through all of the HelpTopic plugins via the plugin manager

    And a few notes from the rest of the discussion:
    - Module developers wanting to distribute help topics with their modules would be able to use the existing admin UI to author their topics, export them using the config manager module, and then save the resulting YML files. These files would be put somewhere other than in config/install or config/optional, so that the plugin manager would be able to read them in.

    Proposed resolution

    Make a patch that preserves the ability for site builders to manage a set of custom help topics, hopefully editing with a familiar user interface and exporting in some way, but makes it possible for module/theme/distribution developers to distribute their help as plugins (which would be translatable but not editable).

    To do this, we would need to the following -- basing this partly on what the Menu Link system in Core does:

    1. [First pass done] Define a plugin base (abstract) class and interface -- see also MenuLinkBase and MenuLinkInterface. The base class will be sort of similar to the existing HelpTopic entity class, but it will not do module and theme dependencies (they are not appropriate for plugins). Also, the base class will have a member variable for the plugin definition as a whole, rather than individual member variables for each config component. So, the get* methods will get stuff from the plugin definition array, not from individual member variables.
    2. [First pass done] Define a plugin manager similar to MenuLinkManager (and an interface and service), but using YamlDirectoryDiscovery for its discovery. It will make use of a plugin class (that extends the plugin base class mentioned above) for this type of help topic plugin (like MenuLinkDefault plugin class).

      The getDiscovery method in the manager will need to be decorated with ContainerDerivativeDiscoveryDecorator so that it will discover our derivatives (see below).

      Module maintainers would put help topics, one per YAML file, in a particular directory, using the same format as the current config entities. This should be done carefully, to allow themes also to provide topic plugins (profiles are included automatically if modules are included). See \Drupal\breakpoint\BreakpointManager or \Drupal\Core\Layout\LayoutPluginManager for two core plugins available to themes.

    3. [First pass done] Plugin translation is usually done in the Discovery class in the getDefinitions() method. So, we would actually need to define our own Discovery class that extends YamlDirectoryDiscovery, because we need the topic title and body to be translated, but the body gets translated in chunks, so the default translation method (which allows only top-level properties to be translated) in YamlDirectoryDiscovery would not work for us (see how it's done in YamlDiscovery::getDefinitions()). We'd need to make sure that help topics that are config entities are not double-translated.

      Alternatively, we could potentially do the translation on the plugin class and not on the discovery class. That seems more straightforward, because the config entities can do their translation their way, and the non-entity plugins can do their translation the way they need to do.

      Note: There is also a related issue that suggests putting plugin-based help topic content into Twig files; that would make translation easier but authoring more difficult, and it's not clear that Twig files is a great place for content. See #2820166: Flexible plugin based help system for more on this idea.

    4. [First pass done] The plugin manager needs to be a service, so we can inject it into things.
    5. [First pass done] Define a plugin deriver similar to MenuLinkContentDeriver (and an interface and service). This will make use of a second plugin class, also extending the base class mentioned above, similar to the MenuLinkContent plugin class (note: there is also a MenuLinkContent class that is the entity).

      The deriver will be discovered during plugin discovery by having its own help topic YAML file, similar to menu_link_content.links.menu.yml. The decorator mentioned above will detect this special format of YAML, know it is a deriver, and set it up.

      Also, the help topic entity should have a method getPluginDefinition() similar to what the MenuLinkContent entity class does. This is called by the deriver, when getting all the plugin definitions derived from config entities, to turn them into fake-ish (derived) plugin definitions.

    6. (postponed until we get this into Core) Figure out POTX (getting the translatable strings into localize.drupal.org). See:
      https://cgit.drupalcode.org/potx/tree/yaml_translation_patterns.yml
      It looks like we would need to add to that file so that within help topic YAML files, we would define which keys are translatable.
    7. [First pass done] Invent a way to display the topics. Entities are fairly straightforward to display, but if each topic is a plugin now, we'd need to create our own viewing infrastructure: make a new controller, routes, access handling, etc.
    8. [First pass done] The HelpSection plugin that generates the section of the admin/help page would need to be converted to use the plugin manager service to find plugins, rather than the entity storage service to find config items.
    9. [Not doing this] Menu links (which is what we're using as a model) allow admins to override static module-provided links. If we wanted to have similar functionality in help topics (not sure if we do or not?), we would need to create a system for overrides similar to what Menu Link does. See the StaticMenuLinkOverrides class.
    10. [First pass done] The help topic entity editing form needs to support putting plugin IDs, rather than entity IDs, into the List On and Related fields. These are currently using an autocomplete that lists entities, but it needs to be changed to use plugins instead.
    11. [First pass done] Existing help topic entity YAML needs to be transformed into plugin YAML (remove the dependencies and locked fields) and put into the plugin YAML directory, so that the Help Topics module is supplying topics as plugins not entities.
    12. [First pass done] Modify the tokens that this module provides so that they link to plugins not entities.
    13. [First pass done] Make sure all the cache tags and contexts are correct. (This is technically part of some of the tasks above, but we've split it off so we can think about it separately.)
    14. [First pass done] Get rid of the "canonical" route for the entities, and replace it by viewing the derived or un-derived plugins instead. It is used a lot in tests (which is already a To Do), as well as on the entity overview page and some other places.
    15. [Done!!] Get all of this working. Write more tests and revise existing tests.

    Remaining tasks

    a) Decide if this is worth doing. The main reasons for wanting to do this:
    - It gives contrib modules (and core too) a way to actively maintain their documentation after the module had been installed.
    - It makes module-provided help untouchable (currently you could unlock and edit the help topics).

    Main reasons for not wanting to do this:
    - Adds complexity
    - Not sure it is desirable to have some topics untouchable

    b) If it's worth doing, make a patch. Test. Commit.

    c) If it is not worth doing, consider whether there are other ways we could address the goals. For instance, we could make some helper functions that would simplify writing a hook_update_N()? There could be a function that would update a topic if the site admin had not customized it, and warn the user running the updates if it had been updated.

    d) To Dos to add to the Core issue and/or separate follow-up issues in the sandbox project once we finish with this:

    • Figure out how to have an Edit link on unlocked entity topic pages, but not on pages that are plugin topics.
    • Create and write the online docs page for this module: https://www.drupal.org/modules/help_topics
    • Make a page that lists all Help Topics on the current site (plugins and entities, both top-level and not top-level). Currently we only list entity topics on the admin page, and only top-level topics on admin/help.
    • Figure out POTX (getting the translatable strings into localize.drupal.org). See:
      https://cgit.drupalcode.org/potx/tree/yaml_translation_patterns.yml
      It looks like we would need to add to that file so that within help topic YAML files, we would define which keys are translatable.

    User interface changes

    Only topics created by administrators or distributed as config entities would be editable. Topics distributed as plugins by modules, themes, and distributions would not be editable.

    API changes

    New plugin system for help topics would be added.

    Data model changes

    Data format for YAML for plugins would be added, which would be pretty much the same as the existing topic config schema.

    CommentFileSizeAuthor
    #65 help_topics-plugin-refactor-2961552-63.patch111.06 KBjhodgdon
    #65 interdiff.txt440 bytesjhodgdon
    #62 help_topics-plugin-refactor-2961552-62.patch111.01 KBjhodgdon
    #62 interdiff.txt27.77 KBjhodgdon
    #60 help_topics-plugin-refactor-2961552-60.patch96.41 KBamber himes matz
    #60 interdiff-2961552-56-to-60.txt17.91 KBamber himes matz
    #56 help_topics-plugin-refactor-2961552-56.patch92.64 KBamber himes matz
    #55 help_topics-plugin-refactor-2961552-54.patch89.06 KBjhodgdon
    #55 interdiff-canonical.patch16.17 KBjhodgdon
    #52 interdiff-misc-to-dos-v2.patch24.51 KBjhodgdon
    #51 interdiff-misc-to-dos.patch20.22 KBjhodgdon
    #45 help_topics-plugin-refactor-2961552-45.patch57.16 KBamber himes matz
    #44 interdiff-tokens-help-section.patch10.21 KBjhodgdon
    #42 help_topics-plugin-refactor-2961552-42.patch47.11 KBamber himes matz
    #40 interdiff-yaml.patch8.99 KBjhodgdon
    #39 interdiff-entity.patch3.56 KBjhodgdon
    #38 interdiff-translation.patch5.43 KBjhodgdon
    #37 help_topics-plugin-refactor-2961552-37.patch34.39 KBamber himes matz
    #34 interdiff-deriver.patch8.16 KBjhodgdon
    #31 interdiff.txt13.23 KBjhodgdon
    #30 help_topics-plugin-refactor-2961552-30.patch20.41 KBamber himes matz
    #18 help_topics-plugin-refactor-WIP-2961552-18.patch28.84 KBamber himes matz

    Comments

    jhodgdon created an issue. See original summary.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    Question: Can themes and distributions have plugins that they distribute, or only modules? If not, I guess themes and distributions can still distribute config...

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Priority: Normal » Critical

    FYI, this is now on the list of "issues to resolve before initial Core commit as experimental module". And I saw on another Ideas issue: "we only have ~8 weeks if we want to ship something in 8.6.".

    So... somewhat urgent. I am upping the priority to Critical, since it needs to happen before we commit to Core.

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented
    Assigned: Unassigned » amber himes matz

    Going to take a stab at this today.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    I made some notes on what I think we need to do for this system... I think we want to do basically what the Menu Link plugins do, except that I think we'll want to set it up so that instead of all the topics being in the same YAML file, we'd want one YAML file per topic, so discovery would be finding topic YAML files in a particular directory with a particular name pattern? See also
    https://api.drupal.org/api/drupal/core%21core.api.php/group/plugin_api/8...
    to read about plugins and discovery in general.

    So, let's see in more detail what menu links do. They have an interface and base class for the menu link plugin:

    https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Menu%21Me...
    https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Menu%21Me...

    Then the core Menu code has one subclass for the in-YAML plugin links:
    https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Menu%21Me...

    And then they have this plugin manager class:
    https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Menu%21Me...
    Note: there is no annotation class, because this is a YAML discovery method, not Annotation discovery method.

    They also have an overrides system, which allows content links to override static (plugin-provided) links:
    https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Menu%21St...
    I don't know if we need something like that or not...

    Then the Menu UI module defines a "Deriver" that lets its in-content menu links show up as menu link plugins:
    https://api.drupal.org/api/drupal/core%21modules%21menu_link_content%21s...
    And to go with that, they also have their own subclass of the plugin class:
    https://api.drupal.org/api/drupal/core%21modules%21menu_link_content%21s...

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    Amber and I looked into this in some depth together today...

    So. We are not quite sure this will work very well:

    a) We would need to do something very similar to what Menu Links in Core do. (see previous comment).

    b) We want modules, themes, and profiles to be able to provide topics. Can themes and profiles provide plugins, or just modules? It looks like MenuLinkManager::getDiscovery() only looks in module directories for its links.menu.yml files.

    c) That aside... The plugin manager for help topics would need to discover the help topics (plugins) provided by modules (at least). The core YamlDiscovery classes do this by looking for one file per module, such as MODULE.links.menu.yml. All of the, in our case, help topics for that module would be provided in that one file. This will be fairly messy for help topics, but probably is doable... but can they have hierarchy like we have for help topics in the config schema? (The body field is a sequence.)

    d) Translatability... In Menu Links, the MenuLinkManager::getDiscovery() method tells YamlDiscovery that title and description properties are translatable. In the case of help topics, what should be translatable is the label (easy enough) and within the body sequence, the text component of each item (not so easy). Is this even possible?

    e) Also, does POTX know about YamlDiscovery's addTranslatableProperty() method, or does it just know about the links.menu.yml files? If it's the latter, we would need to add custom code to POTX to discover the translatable strings within the body of our help topics too. Ugh.

    So... This is nowhere as simple as people were saying it would be. What I think we would need to do:
    - Define a plugin base class and interface
    - Define a plugin manager similar to MenuLinkManager (and an interface and service), using YAML discovery, and a plugin class for this type of help topic
    - If we wanted the plugin YAML files to be less messy, we would need to figure out a new way to discover the topics in separate files, or to read the information from separate files after discovering the basics in one YAML file.
    - Define a plugin deriver similar to MenuLinkContentDeriver (and an interface and service), and it would need to have its own plugin class too. This would be in addition to the config entity classes we already have.
    - Figure out translation and POTX. It looks like YAML discovery only supports translation of top-level properties, and we need translation of a property within a sequence of the body property. Also probably POTX has some custom code that supports translation of menu links and we would need to add to that.

    Is it still worth the trouble?

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    Regarding POTX, I found this
    https://cgit.drupalcode.org/potx/tree/yaml_translation_patterns.yml

    It looks like we would need to add to that file so that within help topic YAML files, we would define which keys are translatable. That doesn't seem too impossible, so we can probably get the strings from help topic plugins into localize.drupal.org at least.

    However, I'm not sure about plugin translations themselves. Once we have those strings in the PO database within a site, and/or translations for them, how will the plugin manager actually translate the help topics? In config, we have a schema, and at each level of the schema you can use data types that are or aren't translatable. But plugins don't have the same thing. It looks like the YAML discovery only lets you define top-level definition components as being translatable -- that really won't work for us, and I just don't really know much about plugin translations... any pointers out there?

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    From @eojthebrave via @Amber Himes Matz: Apparently there is also a directory-based YAML plugin discovery class:
    https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Plugin%21...
    Edit: And the only core user of this class is MigrationPluginManager -- we can at least look at that class for how it calls this.

    That would help with comment 6 item (c), making it at least less ugly. I'm still not sure about translatability though. That class still only looks like it supports having top-level properties be translatable, which isn't going to work for help topics. We can't have full body strings translatable -- they are too big to fit in the translation database (or at least, they could be). See parent issue summary for more details on this.

    jhedstrom’s picture

    jhedstrom
    Primary language English
    Location Portland, OR
    commented

    The cons of a plugin-based approach seem to be numerous in this particular application of it (and drastically more complex than anticipated). I wasn't tied to that approach, but merely wanted to assist in implementation details if that approach was deemed a good one.

    The one big pro of the plugin approach is that it gives contrib modules (and core too) a way to actively maintain their documentation after the module had been installed.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    Yes:

    The one big pro of the plugin approach is that it gives contrib modules (and core too) a way to actively maintain their documentation after the module had been installed.

    Maybe we could address that some other way, like making some helper functions that would simplify writing a hook_update_N()? For instance, there could be a function that would update a topic if the site admin had not customized it, and warn the user running the updates if it had been updated.

    Also I realized I didn't list all the steps we would need to take in #6. We left out:
    - Would need to invent a way to display the topics. Entities are fairly straightforward to display, but if each topic is a plugin now, we'd need to create our own viewing infrastructure: make a new controller, routes, access handling, etc.
    - The HelpSection plugin that generates the section of the admin/help page would need to be converted to use the plugin manager to find plugins, rather than the entity storage to find config items.
    - Menu links (which is what we're using as a model) allow admins to override static module-provided links. If we wanted to have similar functionality in help topics (not sure if we do or not?), we would need to create a system for overrides similar to what Menu Link does.
    - More tests and revised tests.

    dawehner’s picture

    dawehner
    Primary language German
    commented
    Issue summary: View changes
    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Issue summary: View changes
    Related issues: +#2820166: Flexible plugin based help system

    Adding all the existing Q&A and To Do list up to this point to the issue summary.

    Note that I'm removing this part added in the last comment by @daweher as it is not relevant to this proposal (we are not proposing putting the help content in Twig files for this issue):

    • How would you translate help text defined by twig files:
      Twig templates already have build in translation support, for example using: 
          {% trans %}
      You have not created any content types yet. Go to the <a href="{{ create_content }}">content type creation page</a> to add a new content type.
    {% endtrans %}

      This way you can have multiple blocks and not just one

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Issue summary: View changes

    fixing HTML formatting in summary, no content updates.

    tim.plunkett’s picture

    tim.plunkett
    he/him
    Primary language English
    Location Philadelphia
    commented

    Themes cannot currently provide annotated plugins. They can, with the correct code in the plugin manager, provide YAML-defined plugins.

    See \Drupal\breakpoint\BreakpointManager or \Drupal\Core\Layout\LayoutPluginManager for two core plugins available to themes.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Issue summary: View changes

    Thanks for answering that question! I'll put those notes in the summary. Also, noted in Slack that profiles are the same as modules as they are listed as providers by ModuleHandler.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Issue summary: View changes

    @Berdir answered the translatability question in Slack, so putting that into the issue summary.

    No more open questions, except to decide if all this work is worth doing...

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Issue summary: View changes

    Adding a few more details. From discussions in Slack, it is becoming clear we think we should do this.

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented
    StatusFileSize
    new help_topics-plugin-refactor-WIP-2961552-18.patch28.84 KB

    Ok, I'm not too proud to say I'm in over my head and need major help. I've attached a patch against 8.x-2.x with my work so far.

    Some explanation:

    0. I used the word "Plugin" in class/files names so as to distinguish from the work already done to define the Config Entity Type. (Not to overwrite or remove any existing work on the Config Entities right now.)

    1. src/HelpTopicPluginBase.php - I tried to refactor the config entity base class to use methods from PluginBase instead of ConfigEntityBase, like what core/lib/Drupal/Menu/MenuLinkBase.php does. I got stuck on the enforcedDependencies bit, as that is a ConfigEntityBase thing. Not sure how Plugins factor-in module or theme dependencies?

    2. src/HelpTopicPluginInterface.php - Again, not sure about the dependencies...

    3. src/HelpTopicManager.php - Tried to emulate MenuLinkManager. Pretty good and confused about storage. getDiscovery() tries to use YamlDirectoryDiscovery() to find plugins in both module and theme directories.

    4. src/Plugin/Derivative/HelpTopic.php - This is actually the deriver. Not sure if this is the best place to put this. Also, since we're not using Annotations, not sure where to set the deriver class.

    I will totally understand if someone else wants to do this "properly". I am also willing to continue, but as you can see this is slow and awkward for me and I need help. Thanks.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Issue summary: View changes

    Amber and I had another discussion, adding some more notes to the issue summary about what needs to be done...

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented

    Thank you Jennifer for the very helpful discussion today. I should be able to carve out time this week to continue working on this!

    learning++

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    I was thinking some more about translation (not adding this to the issue summary yet, since it's still at the "thought" stage)... In the issue summary, our thought about translation was basically that the Discovery class would be in charge of translating the plugins as they are discovered (i.e., read from YAML files and/or configuration). But I was thinking it might make more sense to put the job of translating on the plugin class instead, on the getTitle() and getBody() methods. The reason I'm thinking this is that the config entity plugins and the non-entity plugins have different ways of being translated, so it makes sense to me that these two types of plugins, which have different classes, would let the plugin class handle the translation.

    Thoughts?

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented

    I don't know. (I'm going to be verbose because I want to achieve some clarity about this.) I think that the reason we were talking about putting translation into the discovery class is because \Drupal\Core\Plugin\Discovery\YamlDiscovery extends \Drupal\Core\Plugin\Discovery\YamlDiscovery which has:

      /**
   * Contains an array of translatable properties passed along to t().
   *
   * @see \Drupal\Core\Plugin\Discovery\YamlDiscovery::addTranslatableProperty()
   *
   * @var array
   */
  protected $translatableProperties = [];

    and

      /**
   * Set one of the YAML values as being translatable.
   *
   * @param string $value_key
   *   The key corresponding to the value in the YAML that contains a
   *   translatable string.
   * @param string $context_key
   *   (Optional) the translation context for the value specified by the
   *   $value_key.
   *
   * @return $this
   */
  public function addTranslatableProperty($value_key, $context_key = '') {
    $this->translatableProperties[$value_key] = $context_key;
    return $this;
  }

    So, that seems to be the model...and that's why we were thinking of handling translation there (in the discovery class).

    What, again, would be left out or potentially double-translated if we handled translation in our discovery class? I'm still confused about the 2 types...the config entity plugin -- that's what a module or theme would provide as a YAML file in a certain directory that our module's discovery would find. But then the non-config-entity plugin - by that do you mean a help topic created with the UI?

    If we handled translation in the (abstract base?) plugin class' getTitle and getBody methods, this would presumably be after discovery and would be acting on either type of plugin. So maybe it does make sense to handle translation here.

    BTW, I'm making at least part of Friday a contrib day, so I'll try and work through this and hopefully have some progress to show either Friday or this weekend.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    For the Menu Link (and probably other) use cases with single-file YAML discovery, the plugin information is just a few properties, which is why it's appropriate to put, for example, a bunch of menu links in a single YAML file. Translation is also easy: there are two translated properties, which is why it seems appropriate to put the translation into the discovery process.

    But in our case, translation is a bit more complex -- we need to translate the title and the text chunks of the body. And the systems for doing this are different, based on whether the plugin information comes from a YAML plugin file (in which case we use the locale module's string translation system to translate the translatable bits), or whether it comes from the config system (which has its own process for overrides, translations being one possible way to override config stuff).

    This is why I think that the plugin class is a better spot to handle translation than the discovery class, because the two types of plugins (one that is discovered from YAML files in a certain directory or with a certain name pattern in modules and themes, and the other that is derived from config entities of a certain type) each will have their own plugin class.

    And I don't think that the abstract base class for plugins should handle translation, because it needs to be done differently for the two cases.

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented

    Ok, I think I see now. That makes sense.

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented

    Patch update best practice question...now that I've uploaded an initial patch, what is best for subsequent updates to it? Do I continue to patch off HEAD of 8.2.x branch? And then create an interdiff between my last patch and the new one?

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented

    Also, should I create a new patch/interdiff for each file as I think I have it ready, or should I wait until it's more to a complete state? There are so many interconnecting parts. While it would be nice to have incremental review, I'm not sure how realistic that is.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    The usual practice is to upload both a patch against the most current 8.2.x branch, and with each new patch, also upload an interdiff if you can. I personally don't always do an interdiff, but I usually would if I think anyone has looked at a previous patch, and if I don't do an interdiff it's because it's a very small patch or very small change, and I would at least put in the comment what changed since the previous patch.

    As far as when to patch... that is totally up to you. Uploading a patch is asking for feedback, so I would say to upload a patch whenever you are hoping for feedback. Anyone who wants to review your patch and has time will do so at that time, and anyone who doesn't have time might skip one or two of your uploads before reviewing a subsequent patch.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    I know you've been at conferences the past week, and I don't want to apply pressure, but ... The deadline for changes to Drupal 8.6 is sometime in the middle of June. I think we would need to allow at least a couple of weeks after we get this patch finalized and committed to the Sandbox to get the Core patch to RTBC (maybe longer) -- there are always nitpicks -- which means we would need to finish this by the end of May at the latest if we want to try for 8.6.x. Otherwise, I guess it's another 6 months to 8.7.x.

    So, I'm wondering how we should proceed:
    - Do you think you can finish this (with reviews/revisions/tests) in 2 weeks? If so, great! I'll be around for hopefully fairly timely code reviews and question answering.
    - If that doesn't seem realistic, is there some way we could break up the work (divide and conquer) to get it done together? I can put some time into coding, and we could review each others' work and coordinate perhaps? (Consider this route only if you would be happy to proceed that way -- I don't want to take this project away from you.)
    - If neither of those applies, we'll have to decide: do we care about 8.6 vs. 8.7? It's probably better to wait and have a good module than force the issue and have lousy code; plus, lousy code is less likely to get committed to Core even as an Experimental module. But it's been a long road already...

    Thoughts?

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented

    Your comment is very timely, and yes, all of this has been on my mind as well.

    My goal is to have something ready for review this week. However, I am also open to sharing the load/coordinating/etc.

    What I think would be most helpful is every-other-day or 2x/week check-ins of some kind. I would *really* love to try and get this in to 8.6. With that in mind, could we coordinate over email and perhaps schedule a hangout for Wednesday?

    I think that would give us both a better sense of how we can both contribute to getting this issue done.

    I realize this is a bit of a vague plan, but like you pointed out, I've been out pretty much all last week and am getting caught up again. :/

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented
    StatusFileSize
    new help_topics-plugin-refactor-2961552-30.patch20.41 KB

    Here's my progress so far. A base class for the plugin, a plugin manager, a deriver, and related goodies (aka interfaces and other stubs).

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    StatusFileSize
    new interdiff.txt13.23 KB

    Here's an interdiff that splits the "chunking" functionality in the help topic entity class into its own utility class. I am running tests locally to see if they will pass; will update the interdiff if necessary. I didn't make a full patch because ... long story. Anyway, you should be able to apply this interdiff to your local repo on top of your current work, since it touches different files. You'll also want to make changes to the PluginBase class like I did in the Entity class to use the new chunker. I'll leave that to you?

    Another note: Normally, set*() methods on entities and plugins return $this, so that they can be chained together. The entity methods were already doing this (because they called some ConfigEntityBase::set() method at the end and returned that value), but not all of them were documented to do so. So, I added that to the entity interface documentation where it was missing.

    For the plugin, you'll want to (a) make the same change in the interface and (b) change the return values so for example, instead of

    +  public function setRelated(array $topics) {
+    $this->pluginDefinition['related'] = $topics;
+    return $this->pluginDefinition['related'];
+  }

    you would do in the last line, return $this;.

    Hope that makes sense... here is the interdiff file.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    This interdiff didn't break anything, or at least the tests all still pass locally. Have at it!

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented

    Interdiff applied just fine. Thanks!

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    StatusFileSize
    new interdiff-deriver.patch8.16 KB

    The other thing I was supposed to work on was the deriver and its baggage. Here's another interdiff for that... A few notes:

    a) I think that the plugin classes for this project belong in a new directory/namespace of src/Plugin/HelpTopic, so I put the new class DerivedHelpTopicPlugin there. I'm expecting that the HelpTopicPluginBase will go there too, as well as the non-base plugin class you're working on.

    b) I changed the name of the deriver class to HelpTopicDeriver.

    c) I added a couple of methods to the help topic entity and its interface to support plugins (similar to what is done in MenuLinkContent).

    d) I haven't tested any of this. It might work... might not... at least it's a start. :)

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Issue summary: View changes

    Updating issue summary... Amber has completed a lot of the tasks, and we have a new one to add.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Issue summary: View changes

    One more update. For now, we're not going to do overrides.

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented
    StatusFileSize
    new help_topics-plugin-refactor-2961552-37.patch34.39 KB

    Here is a patch (diffed from 8.x-2.x HEAD) with all our work in progress so far, including work and todos in comments 31 and 34.

    From today's call:

    - I stubbed out a HelpTopicDefaultPlugin and it could use a look.
    - I removed all the setters from the HelpTopicPluginBase
    - I moved, renamed, and updated namespaces for HelpTopicDefaultPlugin, HelpTopicPluginBase, HelpTopicPluginInterface, HelpTopicPluginManager, and HelpTopicPluginManager php files.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    StatusFileSize
    new interdiff-translation.patch5.43 KB

    Here's an interdiff for one of the tasks I took on from today's call: adding translation to the getLabel() and getBody() methods on the plugin.

    Notes:
    a) Got rid of the stray help_topics.help.topic.yml file at the top level of the repo that was still in the patch.

    b) Added a getLabel() method to the plugin interface and classes. That was missing because it is part of entity base classes.

    c) Modified the plugin interface for the getBody() function to say it is translated.

    d) Modified the getBody() method on the HelpTpoicDefaultPlugin class to translate the body and label.

    e) Added some docs headers to some of these files.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    StatusFileSize
    new interdiff-entity.patch3.56 KB

    Here's an interdiff that fixes up the namespaces on the entity-related classes, since we've moved all of them into the Entity subdirectory. This should be independent of the previous interdiff (the two can be applied in any order on top of the patch in #37). With the two interdiffs and the patch in #37 applied, the existing tests still pass.

    Oh, and one note: In that previous patch, the Plugin interface documentation for getLabel() and getBody() needs to say

    +   * @return string|TranslatableMarkup

    I think I also forgot to put in a use statement for translatable markup in the default plugin class. Probably if I would get my act together and use an IDE it would catch problems like that. :)

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    StatusFileSize
    new interdiff-yaml.patch8.99 KB

    One more interdiff. This one moves all of the provided help topics from being entities to being plugins. I decided I needed to remove the following components from the entity YAML:
    - langcode
    - status
    - dependencies
    - locked
    The rest looked like what we need in the plugin definitions.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Issue summary: View changes

    Updating issue summary for ToDos that are now done.

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented
    StatusFileSize
    new help_topics-plugin-refactor-2961552-42.patch47.11 KB

    No progress from me. But here is a patch incorporating the last 3 interdiffs from @jhodgdon.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Issue summary: View changes

    Adding two more things to the issue summary: tokens and cache tags.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Issue summary: View changes
    StatusFileSize
    new interdiff-tokens-help-section.patch10.21 KB

    Here's an interdiff that does at least a first pass at updating the tokens code, and the help section plugin (the class that makes the list of topics for the admin/help page).

    Note: I added 2 methods to the HelpTopicPluginInterface, which I needed for this patch. I didn't write the methods yet -- they need to go onto the plugin base class, as part of the "Invent a way to display the topics" task that Amber is working on. I named the methods the same as the Entity::toUrl() and Entity::toLink() methods, and they have similar function parameters (minus the entity link type, which doesn't apply to plugins).

    I also documented one method on the plugin manager interface that I used, to define what its return value should be (since I was using it). Hope that's clear!

    Updating the issue summary to mark these two To Dos as done (first pass at least)... It will be "interesting" to see what happens when we try to actually use this...

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented
    StatusFileSize
    new help_topics-plugin-refactor-2961552-45.patch57.16 KB

    This patch has the interdiff from #44 (the help section and tokens) and I added routing for viewing and editing help topics in help_topics.routing.yml and scaffolded a controller in src/Controller/HelpTopicPluginController.php.

    Am I on the right track?

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    Hm... I don't think we need

    +help_topics.help_topic_edit:
+  path: 'admin/help/topic/{id}/edit'
+  defaults:
+    _controller: '\Drupal\help_topics\Controller\HelpTopicPluginController::editHelpTopic'
+  requirements:
+    _permission: 'administer help topics'

    Is that for plugins? We already have edit routing for the entities, but I don't think we need to edit the plugins, as we don't have configuration.

    But yes, the view route and controller look like you're on the right track!

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented

    Oh, right. I knew that and that's what I get for trying to do a patch late at night. Thanks!

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    Part of our discussion yesterday was about PluginManagerBase::createInstance() vs. getInstance().

    I looked into this a bit more today. It looks like createInstance() is the thing to use. The only uses of getInstance() I found in Core were cases where the individual plugin manager class had defined its own getInstance() method, overriding the base method, which in the end called createInstance(). So it looks like the base plugin manager's getInstance() method is probably useless, and we should be using createInstance(), which actually does what we want.

    I'll make the change in the tokens file when I do the other things we discussed in our call. Hopefully later today. You'll need that in the controller you're working on as well.

    tim.plunkett’s picture

    tim.plunkett
    he/him
    Primary language English
    Location Philadelphia
    commented

    #48 is correct, see also
    #1894130: Make \Drupal\Core\Plugin\Mapper\MapperInterface optional for plugin managers
    #2625554: getInstance() fatals on almost all plugin managers

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    Oh good, I was wondering if I needed to file an issue about getInstance() being totally broken... glad it already exists.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    StatusFileSize
    new interdiff-misc-to-dos.patch20.22 KB

    OK, here's the next interdiff, which does the following:
    - #48 (createInstance() vs. getInstance()) for tokens.inc file.
    - Fills out the getTopLevelTopics() and getAllListOn() methods on the plugin manager.
    - Fills out the toUrl() and toLink() methods on the plugin base class.
    - Fixes the topic autocompletes for the entity editor to reference plugins instead of entities, and the entity class and editing form class to assume ListOn and Related are plugin IDs not entity IDs.
    - A few misc docs fixes in the above files.

    Adding another To Do to the issue summary also: we need to get rid of the "canonical" route for the entities, and replace it by viewing the derived or un-derived plugins instead. It is used a lot in tests (which is already a To Do), as well as on the entity overview page and some other places.

    [EDIT: Note that the next interdiff replaces this one!]

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Issue summary: View changes
    StatusFileSize
    new interdiff-misc-to-dos-v2.patch24.51 KB

    I did some poking around and thinking about cache stuff.

    It seems like help topic plugins that are read in from YAML files from modules and themes do not need to worry about cache tags. They are essentially static objects, until the module/theme is upgraded (in which case the page caches should hopefully be invalidated anyway). However, topic plugins that are derived from topic config entities do need cache tags (which we've already defined on the entity).

    So, here's an interdiff to replace the one in #51. The changes are in HelpTopic.php (the entity class), in the code that thinks about cache tags for Related and List On.

    I also fixed up the cache stuff in the plugin base class.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    @Amber Himes Matz -- I can probably take on the new "get rid of the canonical route" task tomorrow, unless you were dying to get into that... it's more of a "revise existing code that I wrote" than a "write some completely new code" task. :)

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented

    @jhodgdon If you have time, great! Please feel free. I will be traveling and busy at Twin Cities Drupal Camp starting tomorrow.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Issue summary: View changes
    StatusFileSize
    new interdiff-canonical.patch16.17 KB
    new help_topics-plugin-refactor-2961552-54.patch89.06 KB

    OK, here's another interdiff, for the "get rid of the canonical route" task. It will only apply on top of the patch in #45 plus the interdiff in #52.

    In case Amber hasn't done any work since the patch in #45, I've also made a new patch, incorporating all of the above.

    I think we're getting close here to having a first pass done -- just need the controller methods, and we can try it out! It will most likely be fairly broken, but hopefully not in an incomprehensible way. :) The main task left besides the controller methods is revising and adding tests.

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented
    StatusFileSize
    new help_topics-plugin-refactor-2961552-56.patch92.64 KB

    Ok! Here is a patch with the plugin refactor working, but without tests. This is ready for review and manual testing. Next step after that will be writing or refactoring tests, more manual testing, and documentation.

    Thanks to @jhodgdon for the pair programming session today where we went through all the errors we could find and got everything working as expected.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    Updating the issue summary with task status.

    So... A few thoughts on this patch:

    a) I think we can get rid of one "entity" in the URL for the entity topics, and make them just "entity:ENTITY_ID", by changing the getPluginId() method on the entity. That will make the URL much more intuitive. I'll take care of that when I work on (b).

    b) As noted, the next step is tests. I will work on getting the existing automated tests working again (I'm sure some are failing), and writing some new tests and/or revising existing ones to test the plugin portion of the module.

    c) Random other things to fix (hopefully Amber will work on these):

    1. In help_topics.module, a typo in the hook_help():
      "You can add, edit, delete, and translate configure help topics ..." should be "configurable help topics" here.
    2. To test manually: Look at the module overview for this module from admin/help and verify the links on the page work and the text makes sense.
    3. See if the IDE has any warnings that we should address, like unused "use" statements, methods not existing [due to not having @var comments], etc.]
    4. Classes/methods that are missing doc blocks entirely:
      - src/Controller/HelpTopicPluginController -- main class and two methods are missing doc blocks.
      - src/Plugin/HelpTopic/DerivedHelpTopicPlugin -- create() method.
    5. Doc blocks that could use some editing:
      - src/Controller/HelpTopicPluginController -- pluginManager member variable: 
      +  /**
+   * The Help Topic plugin manager.
+   *
+   * We use this to get the help topic plugin.

      I don't think we need that second line. Seems redundant.

      - constructor method in that same class:

      +  /**
+   * Constructor.
+   *
+   * @param \Drupal\help_topics\Plugin\HelpTopic\HelpTopicPluginManagerInterface $help_topic_plugin_manager
+   *  The help topic plugin manager service. We're injecting this service so that
+   *  we can use it to access the help topic plugins.
+   */
+  public function __construct(HelpTopicPluginManagerInterface $help_topic_plugin_manager, Token $token) {

      This needs to start with a verb, like "Constructs a ...". Also missing the $token parameter docs. Also we don't probably need the sentence that says "We're injecting...".

      -src/Plugin/HelpTopic/HelpTopicPluginManager

      + /**
      + * Gets the plugin factory.
      + *
      + * @return \Drupal\Component\Plugin\Factory\FactoryInterface
      + */
      + protected function getFactory() {

      The @return needs a documentation line.

      -src/Plugin/HelpTopic/HelpTopicPluginManagerInterface

      +/**
+ *
+ * Defines an interface for managing help topics and storing their definitions.
+ */
+interface HelpTopicPluginManagerInterface extends PluginManagerInterface {

      Extra blank line before the first docs line here.

    6. There are some vertical whitespace issues here and there, such as two blank lines between methods, missing newlines at ends of files, and missing having a blank line after the end of the last method in a class (between the last } of the last method, and the closing } of the class, there is supposed to be a blank line).

    d) Things to add to the list of post-beta:
    - Figure out how to have an Edit link on unlocked entity topic pages, but not on pages that are plugin topics.

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented

    Yes, I will work on "c)" from comment #57.

    Thanks for the review!

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented

    Great! Let's both plan on uploading interdiffs with our work. I do not think we will conflict.

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented
    StatusFileSize
    new interdiff-2961552-56-to-60.txt17.91 KB
    new help_topics-plugin-refactor-2961552-60.patch96.41 KB

    Ok, I had a bit of a brain freeze about interdiffs, but I think I did it. I also created a patch just in case.

    In a nutshell, I handled c) from #57. Anything labeled [not fixed] is a question I have.

    Verbose notes:

    admin/help/help_topics (module overview):
    - online documentation for the Help Topics module link is a placeholder
    - [TODO] Create and write that page: https://www.drupal.org/modules/help_topics

    admin/config/development/help (Help topics)
    - Should there be a page that lists all Help Topics on the current site? This page only lists help topic entities.
    - We say in http://help-sandbox.local/admin/help/topic/help_system_building “2. …Plugin-based topics cannot be edited, and are accessible from the main Help page.” But this will only list top-level topics.

    IDE complaints:
    - [fixed] Fixed namespace complaints in src/HtmlChunker.php
    - [fixed] ‘info’ not found which caused the theme autocomplete to not work. Fixed by calling getName() instead. In src/Controller/AutocompleteController.php
    - [fixed] Method isLocked() not found in Drupal\Core\Entity\EntityInterface in HelpAccessControlHandler::checkAccess. Added @var \Drupal\help_topics\Entity\HelpTopicInterface $entity
    - [fixed] EntityListBuilder::getLabel is deprecated. Use $entity->label() instead. Fixed in src/Entity/HelpListBuilder.php.
    - [fixed] A bunch of method not found complaints in HelpListBuilder::buildRow(). Fixed by adding an @var inside the method to show the IDE that $entity is a HelpTopicInterface.
    - [fixed] More method not found complaints in HelpLockForm::submitForm(). Refactored, setting $entity = $this->entity and added a @var comment.
    - [fixed] Updated the @var with the correct namespace in HelpTopicForm::form().
    - [fixed] Added @var to HelpTopicForm::copyTopicFieldsToEntity() to placate IDE
    - [fixed] More method not found complaints in HelpTopicForm::save(). Refactored, setting $entity = $this->entity and added a @var comment.
    - [fixed] More method not found complaints in HelpUnlockForm::submitForm(). Refactored, setting $entity = $this->entity and added a @var comment.
    - [fixed] Added @var to HelpTopicDeriver::getDerivativeDefinitions() and fixed case of subsequent getPluginId method in src/Plugin/Deriver/HelpTopicDeriver.php
    - [not fixed] HelpTopicSection::getCacheTags is returning $this->pluginManager->getCacheTags() but getCacheTags() isn’t in the Plugin Manager, it’s in the HelpTopicPluginBase class. So the method isn’t found. Not sure how to fix.
    - [fixed] Removed unused class from use statements in src/HelpTopic/HelpTopicPluginBase
    - [not fixed] HelpTopicPluginManager::__construct() - missing parent __construct() call, but we don’t want to call that constructor because it assumes annotation-based discovery. Not sure what the resolution is here.
    - [fixed] Various vertical line spacing issues.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Issue summary: View changes

    Wonderful! I will work on the tests today, starting from your latest patch.

    I'm adding the To Dos from your list and mine in #57 to the issue summary.

    Regarding the question about a page listing all help topics... I guess that could be useful. I'll add that to the issue summary here as a possible future To Do.

    Regarding the constructor on HelpTopicPluginManager, I think we just have to let the IDE complain, because we do not want to call the parent constructor.

    Regarding getCacheTags(), I will look into that today.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Issue summary: View changes
    Status: Active » Needs review
    StatusFileSize
    new interdiff.txt27.77 KB
    new help_topics-plugin-refactor-2961552-62.patch111.01 KB

    OK, I have made a new patch! I think we are now ready for reviews, as all the tests pass now! Updating the issue summary too, and the status to Needs Review (finally!). I'm going to be out backpacking for the next few days... probably some other US people are too, due to the holiday... but if anyone is around and wants to review this patch, that would be great!

    @Berdir, @jhedstrom, @tim.plunkett, and @dawehner -- I'm especially thinking of you, as you all showed some interest in this. Anyone else is also welcome to review! I will also give the code and comments a full review again before we get this into the Core patch.

    For anyone who is reviewing this patch out of the context of the previous patches, the issue summary goes into some detail about what we had to do to get this working.

    For Amber/me who have been working on this, or anyone else who has been following along the whole time -- what this interdiff patch does:
    - Modify tests to use the plugin system, new URLs, etc.
    - Fix the cache tags on the help page section (see #60). Also, the cache tags for the view topic pages were not quite right, so I fixed them up too.
    - Change the URL for config entity help topics to use the ID entity:ENTITY_ID instead of entity:entity.ENTITY_ID.
    - The URLs for autocomplete routes had "config-help" in them, which was the old machine name of this module, so I changed them.
    - Reverse this change in the autocomplete controller from the patch/interdiff in #60:

    -      $themes[$name] = $extension->info['name'];
+      $themes[$name] = $extension->getName();

    This was causing the Admin test (where it tests autocompletes) to fail. The reason is that $extension->getName() returns the machine name of the theme, not the displayed name, which is what we want for the autocomplete. I think we'll just have to live with the IDE warning. See
    https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension...
    which states that the theme handler puts a bunch of properties into the Extension object, which are unfortunately undocumented on Extension and are not available from methods as far as I can tell.
    - The "list on" feature wasn't working; now it is.

    • jhodgdon committed 34188ef on 8.x-2.x 
      Issue #2961552 by jhodgdon, Amber Himes Matz: Refactor using a plugin...

    jhodgdon credited Berdir.

    jhodgdon’s picture

    jhodgdon
    she/her
    Primary language English
    commented
    Status: Needs review » Fixed
    StatusFileSize
    new interdiff.txt440 bytes
    new help_topics-plugin-refactor-2961552-63.patch111.06 KB

    We are trying to get this into Drupal Core 8.7, so I decided since both Amber and I worked on this, it had probably undergone enough review. I just reviewed the patch again and found one line that needed a trivial fix (see interdiff). So, I'm committing the patch attached here to the Sandbox project, and will go make a new Core patch on #2920309: Add experimental module for Help Topics

    amber himes matz’s picture

    amber himes matz
    she/her
    Primary language English
    Location Portland, OR USA
    commented
    Assigned: amber himes matz » Unassigned

    Status: Fixed » Closed (fixed)

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