Problem/Motivation

When you go to https://git.drupalcode.org/project/feeds/-/pipelines and then on "stages", choose eslint or styleint, you will see a list of coding issues.
At the time of writing you can find these on https://git.drupalcode.org/project/feeds/-/pipelines/109598

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork feeds-3425219

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

MegaChriz created an issue. See original summary.

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

franz’s picture

Eslint is passing now.

franz’s picture

Status: Active » Needs review

Passing both now.

megachriz’s picture

Status: Needs review » Needs work
StatusFileSize
new115.58 KB
new119.12 KB
new114.77 KB

Thanks for helping! Unfortunately, the JS in modules/log/js/feed_type_settings.js is no longer doing what it should do.

In the vertical tab 'Log settings' on a feed type page it should tell the user whether or not logging is enabled for the feed type:

After the changes from this issue, the summary on the vertical tab 'Log settings' always says that logging is disabled, even if it is enabled.

franz’s picture

Status: Needs work » Needs review

I fixed that portion of code that was "vanillaed", it seems to be working now.

ptmkenny’s picture

I just tested this and "Logging disabled" and "Logging enabled" are reflected properly.

However, I noticed an unusual behavior that I would like to confirm.

With logging enabled, when both "Log message" and "Log item" are checked, if you uncheck "Log message" first, then "Log item" is disabled and you cannot uncheck it.

However, if you uncheck "Log item" first, then you can uncheck "Log message" as well.

This is inconsistent and does not make sense to me.

However, this behavior seems to be the same on the 3.x branch of the module, so it may be a separate issue.

Not marking RTBC because of the above concern.

  • MegaChriz committed eb87e9a4 on 8.x-3.x authored by franz
    Issue #3425219 by franz, MegaChriz, ptmkenny: Fixed eslint and styleint...
megachriz’s picture

Status: Needs review » Fixed

@ptmkenny

With logging enabled, when both "Log message" and "Log item" are checked, if you uncheck "Log message" first, then "Log item" is disabled and you cannot uncheck it.

But after you save the settings, "Log item" will be unchecked. The idea is that if you disable logging for a certain operation, you cannot log items for it. So that's why the "Log item" checkbox will become disabled.
I do see that there's an issue that when both checkboxes are not checked on page load, when you enable logging for a certain operation, the "Log item" checkbox remains disabled. But when unchecking and checking logging for an operation, you can control "Log item" again.

I think changing the form for the log settings is something that would better be addressed in a follow-up issue, but thanks for testing @ptmkenny :)

I merged the code.

ptmkenny’s picture

@MegaChriz Thanks for the explanation!

Status: Fixed » Closed (fixed)

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