Add setDatabaseResult(EmptyRecycleBinResult) and setDatabaseResult(List<EmptyRecycleBinResult>)

[TrangOul]

closes #804

[TrangOul]

WIP - unit tests needed!

[TrangOul]

Swapped with DeleteResult to maintain alphabetical order.
Similar changes in other classes.

[jongpie]

@TrangOul overall this looks great! I think having completeness of all of the Database....Result objects is a great idea, thanks for working on this. I think the only missing pieces are:

• The extra unit tests that you mentioned here
• The class LoggerMockDataCreator didn't seem to change, but I see that you added a new method in the docs file LoggerMockDataCreator.md

There are also a couple of steps that I'll have to manually run to generate the new package version, so if you'd like any help wrapping up the unit tests/anything else, just let me know!

[TrangOul]

@jongpie, how do you generate unit tests? Right now you have 2 message formats × 7 logging levels × 7 Database Results (some tests are still missing - MergeResult) - quite a lot.... Do you have a some kind of a generator (a script maybe)? I did most of the editing using cumbersome regexes (matching entire test methods with multiple capture groups), but maybe there is an easier way?
Anyway, please strongly consider refactoring the test class and eliminating repeated code in favor of parametrized methods.

[jongpie]

@TrangOul great question - I used to have some macros/keyboard shortcuts to help me with writing those test methods in Logger_Tests, but I haven't added new method overloads in a while, so I haven't improved that process. The Logger_Tests class is also one of the two oldest files in the repo (Logger being the other one), so there's still some very old approaches used for validating some behavior, all of which I think can (eventually) be improved. I've logged #816 to track this - I'll use that issue to "modernize" some of the older tests + find a way to make it easier to add new tests in the future.

In the meantime, I really appreciate you working on the tests - I'm working on reviewing your latest changes, and I'm hoping I can merge & release this either later this week or next week.

[jongpie]

@TrangOul I saw you pushed a few more commits - let me know if you're planning to push any more changes.

[TrangOul]

@jongpie if it's blocking you (because of more code to review), I won't push more for now. I'll just create a new PR for subsequent changes (if any).

[jongpie]

@TrangOul that would be great - I'd like to get this merged ASAP so I can also finish a few other releases this week. But if there are any other changes that you'd like to still push to this branch first, please let me know, and I can review/merge it when you're done. At the moment, there are several test failures in Logger_Tests, but I'm happy to fix those if you'd like, just let me know.

[TrangOul]

@jongpie , I think I'm done for now.
Please let me know what needs to be fixed (or you can do it yourself, if it's something small).

[jongpie]

@TrangOul this is ready to go! I'm working on merging & releasing this now 🥳 I've added some commits with a few changes:

• Fixed some test failures in Logger_Tests
  • Some failures were easy fixes due to some very small issues in the setup of some of the new test methods
  • A few of the failures were happening in existing test methd (related to saving date via saveLog()), caused by some downstream exceptions within the class LogEntryHandler. It's something I'll revisit in the future to optimize, but for now, I made a few quick workarounds in the Logger_Tests methods to avoid the exceptions
• Added a few more tests in LogEntryEventBuilder_Tests for the new Database.EmptyRecyleBin overloads
• Reverted some of your changes:
  • Reverted casing changes UserInfo.getUsername()
  • Reverted back to System.now() and System.today()
• Small scope creep: Fixed some existing code that caused PMD scan violations in newer versions of PMD/sf code analyzer

[jongpie]

I've just finished releasing this as release v4.15.2. Thanks again for your help!

[TrangOul]

@jongpie, why? It was correct, according to the Salesforce API.
The IDE complains about the current one:

[jongpie]

@TrangOul I've never seen that warning in VS Code, but maybe I've disabled whatever option that is - do you know what plugin/feature controls that warning message?

Personally, I've always considered the user of getUserName() (capital N) a bug in Apex/Salesforce docs. I think getUsername() should be considered the correct casing - it's consistent with the rest of the platform, and it provides clarification that it is referring to the "username", and the "user's name".

• Everywhere else in Salesforce (and from what I've seen in other systems too/software engineering in general), it's always spelled Username with a lowercase n. The only place I'm aware of in Salesforce that uses the capital N is that method name. And since it returns the value of the field User.Username, I think the method name should use the same casing.

  • User.Username field has lowercase n in the field API name, the field label, and the corresponding docs

    

  • Salesforce login pages (including Community Cloud/Experience Cloud, etc) all

    

I've also worked on multiple teams (at different companies) where using getUserName() is too ambiguous & confusing for people new to Apex (or anyone unfamiliar with the UserInfo class), occasionally resulting in bugs.

• Some people (incorrectly) thought UserName referred to the user's name (i.e., the User.Name field that combines User.FirstName and User.LastName
• Some people (correctly) thought UserName referred to the user's username (i.e., the User.Username field)

I think both are valid thoughts/guesses with the UserName casing - Salesforce uses User as a prefix on large number of standard fields (listed below), so I think there will always, inevitably, be some confusion if getUserName() refers to User.Name or User.Username

• User.UserRoleId
• User.UserType
• 12 fields that with UserPermissions..., like User.UserPermissionsMobileUser
• 81 fields that start with UserPreferences...., like User.UserPreferencesUserDebugModePref

But I've personally never had a situation where using getUsername() is unclear/confusing.

[TrangOul]

@jongpie, I'm using WebStorm with Illuminated Cloud 2. Here are my settings:

Unfortunately, Salesforce is inconsistent when it comes to letter casing. Another example: SObject:

(in my own code I use sobject at the beginning of the name and SObject later, for example getSObjectApiName(sobj); IMO sObj looks goofy)

It's even more annoying, since Apex is case-insensitive and such inconsistencies could be fixed in one of the release updated without causing any regression.

I agree with your reasoning that Username = username (technical) and UserName = user's name (user-friendly), and I also stick to "username" as a single word in my own class/function/variable names (and also correct them in code reviews).
Nevertheless, I'm sticking to what SF API uses because it simplifies my workflow - I just use automatic formatting (which, obviously, does much more that changing the case).