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

feat: more source member fields #704

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

mshanemc
Copy link
Contributor

@mshanemc mshanemc commented Nov 11, 2024

What does this PR do?

new fields for getChanges (the RemoteChange type)

  • @W-17059652@ revisionCounter (straight from SourceMember)
  • changedBy (it's a userId on SourceMember, so we resolves it to the user's Name, like Shane McLaughlin, not like a username] and cache the results of the user query
  • @W-17146388@ the change type (add, modify, delete) derived from SourceMember.
  • @W-17032000@ memberIdOrName (straight off of sourceMember

Makes no changes to the output of the CLI preview commands, but the additional fields might be useful there (ex: I want to only see my changes, sorting change in order or recency (revisionCounter) etc)

The preview command is already doing some of this change type calculation.

Some of these fields will appear in the local tracking files, but no changes to existing fields are made.

xNUT failure is because PDR is asserting things about the tracking files that are changed as part of this PR. If approved, I'll write a change for that NUT

@mshanemc mshanemc requested a review from a team as a code owner November 11, 2024 21:45
@@ -46,16 +46,16 @@ const isSpecialAuraXml = (filePath?: string): boolean =>

// things that never have SourceMembers
const excludedKeys = [
'AppMenu__Salesforce1',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the key delimiter. I think this might help some of the tracking issues with namespaced metadata, though the API drops some namespaces, too and this won't fix that.

'IsNameObsolete',
'RevisionCounter',
'IsNewMember',
'ChangedBy',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

is an ID field, not a real lookup. And it's 15-char.

ApexClass__MyClass: {
serverRevisionCounter: 3,
ApexClass###MyClass: {
RevisionCounter: 3,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since I added a few more fields from the server, it seemed a good time to use the SourceMember's fields directly instead of a "let's rename them all in the json file"

@@ -206,16 +227,14 @@ export class RemoteSourceTrackingService {
// to sync the retrieved SourceMembers; meaning it will update the lastRetrievedFromServer
// field to the SourceMember's RevisionCounter, and update the serverMaxRevisionCounter
// to the highest RevisionCounter.
public async retrieveUpdates({ sync = false, cache = true } = {}): Promise<RemoteChangeElement[]> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

nothing was using this and this isn't a public API

@@ -458,7 +466,7 @@ ${formatSourceMemberWarnings(outstandingSourceMembers)}`
const matchingKey = sourceMembers.get(key)
? key
: getDecodedKeyIfSourceMembersHas({ sourceMembers, key, logger: this.logger });
this.sourceMembers.set(matchingKey, sourceMember);
this.sourceMembers.set(matchingKey, { ...sourceMember, MemberName: decodeURIComponent(sourceMember.MemberName) });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

check the logic here: let's always use a decoded key; the get already checks for the encoded version if it doesn't find a match

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should be safe, both functionally and performance.

const conn = this.org.getConnection();
const queryResult = await queryFn(conn, query);
await updateCacheWithUnknownUsers(conn, queryResult, this.userQueryCache);
const queryResultWithResolvedUsers = queryResult.map((member) => ({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

converting the userId to a real name; query only those we haven't already seen and cache to avoid next time

}

private async write(): Promise<void> {
const lockResult = await lockInit(this.filePath);
await lockResult.writeAndUnlock(
JSON.stringify(
{
fileVersion: CURRENT_FILE_VERSION,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

a fileVersion makes changes easier next time

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.

2 participants