Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add logging information when a dataset has already been imported #4377

Merged
merged 3 commits into from
Jan 13, 2025

Conversation

zedmonds96
Copy link
Contributor

Fixes #4334

Describe your changes

Add logging facilities to Drupal\datastore\Plugin\QueueWorker\ImportQueueWorker::processItem() such that it informs the user that the dataset already exists and will not be imported.

QA Steps

  • Import a dataset
  • Attempt to re-import the same dataset
  • Confirm log message notifying that the dataset was not re-mported

Checklist before requesting review

If any of these are left unchecked, please provide an explanation

  • [N/A] I have updated or added tests to cover my code
  • [N/A] I have updated or added documentation

@zedmonds96 zedmonds96 changed the title added logging information when a dataset has already been imported Add logging information when a dataset has already been imported Jan 7, 2025
@zedmonds96 zedmonds96 self-assigned this Jan 7, 2025
Copy link
Member

@dafeder dafeder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zedmonds96 can we follow the pattern for other tests on this class, to check for the logger execution as well? If you look at ImportQueueWorkerTest::testErrorPath() I think you'll see a pattern you can use in ::testProcessItemAlreadyImported().

@zedmonds96
Copy link
Contributor Author

@dafeder is this cypress test failure related to my changes?

@dafeder
Copy link
Member

dafeder commented Jan 10, 2025

I'm sure it's a random failure we keep getting on that test. Can you rerun it?

@zedmonds96 zedmonds96 removed their assignment Jan 13, 2025
@zedmonds96 zedmonds96 requested a review from dafeder January 13, 2025 12:54
@zedmonds96
Copy link
Contributor Author

@zedmonds96 can we follow the pattern for other tests on this class, to check for the logger execution as well? If you look at ImportQueueWorkerTest::testErrorPath() I think you'll see a pattern you can use in ::testProcessItemAlreadyImported().

I think I added this check correctly, but not 100% sure.

@dafeder dafeder merged commit b515526 into 2.x Jan 13, 2025
11 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide logging information when a dataset has already been imported
2 participants