Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Added query selector classes, fixed some recurring bugs #314

Merged
merged 15 commits into from
May 23, 2022

Conversation

jongpie
Copy link
Owner

@jongpie jongpie commented May 20, 2022

Enhancements
This PR is primarily focused on adding some optimizations & cleanup as pre-work to implementing some stellar suggestions from @LawrenceLoz that will be implemented as part of #311 (originally discussed in #303)

  • Added new Apex selector classes LoggerEngineDataSelector and LogManagementDataSelector to centralize (almost) all queries throughout the codebase
  • Added new Apex class LoggerCache to centralize caching of query results

Bugfixes

  • Possibly fixed an issue discussed in Separate Queueable runs are collected into one Log__c record #310 with @reitffunk by reverting to only using a UUID for the transaction ID (instead of using System.Request.getCurrent().getRequestId())
  • Fixed Failing tests due to DUPLICATE_USERNAME #313 reported by @jverelst by switching to using a combination of Logger's transaction ID + an incremented counter to dynamically creating unique usernames during tests. This should ensure that the generated username is unique both within a transaction, and across all Salesforce orgs
  • Fixed LoggerEmailUtils_Tests failed #276 (again) by moving when an email is added to the constant LoggerEmailSender.SENT_EMAILS, which was previously causing test failures in orgs with email deliverability disabled.
    • Scratch orgs also can't have deliverability disabled (very frustrating), so I also separately changed email deliverability to 'System Email Only' in the pkg demo org that's used in the pipeline, which should help ensure everything is working going forward 🤞
  • Added missing access to the class LoggerSObjectMetadata to permission sets 'LoggerAdmin' and 'LoggerLogViewer'
    • This solves an issue in orgs where Apex access is enforced - without the class access, the Logger Settings tab/LWC would throw an error on page load
  • Changed the approach used to originally to solve Site Guest User no longer creating logs #219 by using the User Type (UserInfo.getUserType()) of the user's profile, which seems to be a more stable option than the old approach of checking the profile's user license name
    • This was originally reported months ago by @solo-1234, who encountered this issue again. I was not able to reproduce the issue, but using User Type seems like a better approach anyway, and will hopefully resolve any lingering issues with the Guest User
    • I tried to find an option that didn't rely on a hardcoded String, but so far, have not found any other way to determine it

Metadata Cleanup

  • Deleted some deprecated LoggerParameter__mdt records - these were deprecated in v4.7.1, but I forgot to remove them from the repo/package
  • Eliminated layer-specific test suites within core - as of v4.7.1, the tests are super fast anyway, so the 'LoggerCore' test suite covers everything + makes maintenance easier
  • Moved the 'LoggerLogCreator' permission set back to being part of the logger-engine layer

Async Failures plugin

  • Added a missing CMDT record LoggerSObjectHandler.BatchApexErrorEvent for the async failures plugin (and created a new package version that includes the CMDT record)

jongpie added 12 commits May 10, 2022 17:44
…DataSelector to centralize (almost) all queries, added LoggerCache + platform cache partition for caching
… class

- Eliminated platform cache (at least for now) - I'll revisit in a separate branch/PR
- Changed 2 selector classes to use singleton pattern (instead of all static methods)
- "Finished" integrating selector classes into existing codebase
- Added tests classes for 2 selector classes
- Added LogViewerController.cls to use within logViewer LWC and removed Logger.getLog()
… tests are super fast anyway, so the LoggerCore test suite covers everything + makes maintenance easier
…recated in v4.7.1, but I forgot to remove them from the repo
…a UUID for the transaction ID (instead of using System.Request.getCurrent().getRequestId())
…constant SENT_EMAILS

Scratch orgs also can't have deliverability disabled (very frustrating), so I also separately changed email deliverability to 'System Email Only' in the pkg demo org that's used in the pipeline, which should help ensure everything is working going forward 🤞
…LoggerAdmin and LoggerLogViewer, moved LoggerLogCreator perm set back to being part of the logger-engine layer
…nter for dynamically creating unique usernames during tests
…pped working due to a change in some orgs of the value used for the Guester User LIcense field on the UserLicense object. I've switched to instead using the User Type (UserInfo.getUserType()), which seems to be a more stable option that's less likely to change

- I tried to find an option that didn't rely on a hardcoded String, but so far, have not found any other way to determine it
@jongpie jongpie had a problem deploying to Base Scratch Org May 20, 2022 21:28 Failure
@jongpie jongpie had a problem deploying to Experience Cloud Scratch Org May 20, 2022 21:28 Failure
@jongpie jongpie had a problem deploying to Base Scratch Org May 20, 2022 21:49 Failure
@jongpie jongpie had a problem deploying to Experience Cloud Scratch Org May 20, 2022 21:49 Failure
@jongpie jongpie temporarily deployed to Base Scratch Org May 20, 2022 22:17 Inactive
@jongpie jongpie temporarily deployed to Experience Cloud Scratch Org May 20, 2022 22:17 Inactive
@jongpie jongpie temporarily deployed to Base Scratch Org May 21, 2022 00:43 Inactive
@jongpie jongpie temporarily deployed to Experience Cloud Scratch Org May 21, 2022 00:43 Inactive
@jongpie jongpie temporarily deployed to Demo Org May 21, 2022 00:51 Inactive
@jongpie jongpie temporarily deployed to Demo Org May 21, 2022 01:00 Inactive
@jongpie jongpie marked this pull request as ready for review May 23, 2022 14:42
…aked a few tests that failed only in the pkg demo org
@jongpie jongpie force-pushed the feature/add-query-selector-classes branch from e067c1b to 4ac7314 Compare May 23, 2022 15:09
@jongpie jongpie temporarily deployed to Experience Cloud Scratch Org May 23, 2022 15:12 Inactive
@jongpie jongpie temporarily deployed to Base Scratch Org May 23, 2022 15:12 Inactive
@jongpie jongpie temporarily deployed to Demo Org May 23, 2022 15:36 Inactive
@jongpie jongpie temporarily deployed to Demo Org May 23, 2022 15:53 Inactive
@codecov
Copy link

codecov bot commented May 23, 2022

Codecov Report

Merging #314 (4ac7314) into main (7c6f49c) will decrease coverage by 0.08%.
The diff coverage is 91.05%.

❗ Current head 4ac7314 differs from pull request most recent head ee8bd63. Consider uploading reports for the commit ee8bd63 to get more accurate results

@@            Coverage Diff             @@
##             main     #314      +/-   ##
==========================================
- Coverage   95.52%   95.44%   -0.09%     
==========================================
  Files          45       49       +4     
  Lines        4651     4738      +87     
  Branches       92       92              
==========================================
+ Hits         4443     4522      +79     
- Misses        205      213       +8     
  Partials        3        3              
Flag Coverage Δ
Apex 96.00% <91.05%> (-0.12%) ⬇️
LWC 91.81% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ore/main/log-management/lwc/logViewer/logViewer.js 86.76% <ø> (ø)
...ore/main/logger-engine/classes/LoggerDataStore.cls 100.00% <ø> (ø)
...-logger/core/main/logger-engine/classes/Logger.cls 94.60% <40.00%> (-0.38%) ⬇️
...er/core/main/log-management/classes/LogHandler.cls 98.55% <71.42%> (-0.02%) ⬇️
...in/log-management/classes/LogEntryEventHandler.cls 95.01% <82.35%> (-0.52%) ⬇️
...ain/logger-engine/classes/LogEntryEventBuilder.cls 97.02% <86.04%> (+0.43%) ⬆️
.../main/log-management/classes/LoggerEmailSender.cls 93.25% <87.50%> (+0.15%) ⬆️
...logger-engine/classes/LoggerEngineDataSelector.cls 87.71% <87.71%> (ø)
...re/main/log-management/classes/LogEntryHandler.cls 95.34% <100.00%> (-0.24%) ⬇️
...g-management/classes/LogManagementDataSelector.cls 100.00% <100.00%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c6f49c...ee8bd63. Read the comment docs.

@jongpie jongpie merged commit c866d36 into main May 23, 2022
@jongpie jongpie deleted the feature/add-query-selector-classes branch May 23, 2022 18:08
@jongpie jongpie mentioned this pull request May 23, 2022
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failing tests due to DUPLICATE_USERNAME LoggerEmailUtils_Tests failed
2 participants