Skip to content
This repository has been archived by the owner on Mar 18, 2024. It is now read-only.

fix(profile): add ability to reset cache when the command is executed against an org multiple times #1422

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

Conversation

azlam-abdulsalam
Copy link
Contributor

@azlam-abdulsalam azlam-abdulsalam commented Oct 13, 2023

The PR adds an additional flag --resetcache to merge/reconcile which provides ability to reset cache
as required among subsequent runs

fixes #1413

Summary generated by Reviewpad on 17 Oct 23 01:45 UTC

This pull request includes two patches.

The first patch fixes an issue in the profile merge and reconcile commands. It adds a new flag '--resetcache' which allows users to reset the cache and retrieve the latest profile permissions from the org. This fix addresses issue #1413.

The second patch fixes a formatting issue with the prettier in the sfpowerkit utility file.

Overall, these patches enhance the profile merge and reconcile commands and improve the formatting of the sfpowerkit utility.

Checklist

All items have to be completed before a PR is merged

  • Adhere to Contribution Guidelines
  • Updates to Decision Records considered?
  • Updates to documentation at DX@Scale Guide considered?
  • Tested changes?
  • Unit Tests new and existing passing locally?

… against an org multiple times

The PR adds an additional flag --resetcache to merge/reconcile which provides ability to reset cache
as required among subsequent runs

fixes #1413
@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (7c5d9d4) 46.80% compared to head (6b32c0f) 46.80%.
Report is 1 commits behind head on main.

❗ Current head 6b32c0f differs from pull request most recent head b2ac562. Consider uploading reports for the commit b2ac562 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1422   +/-   ##
=======================================
  Coverage   46.80%   46.80%           
=======================================
  Files          70       70           
  Lines        2662     2662           
  Branches      302      302           
=======================================
  Hits         1246     1246           
  Misses       1414     1414           
  Partials        2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…get-org-or-include-the-org-name-into-the-cache
@reviewpad
Copy link

reviewpad bot commented Oct 15, 2023

Reviewpad Report

‼️ Errors

  • Unconventional commit detected: 'Merge branch 'main' into 1413-reset-the-sfpowerkit-on-a-different-target-org-or-include-the-org-name-into-the-cache' (c607b3a)

⚠️ Warnings

  • Please rebase your pull request on the latest changes

@@ -65,7 +65,7 @@ export default class Retrieve extends SfpowerscriptsCommand {
folders.push(...argFolder);
}

Sfpowerkit.initCache();
Sfpowerkit.initCache(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

@azlam-abdulsalam
Why did you only optionally reseted the cache for the other two commands and forced it for the retrieve? Why not uniformly? The error can occur with every command. And is there anything against not flushing the cache when changing the org? Or to build the cache key with alias?

Copy link
Contributor

Choose a reason for hiding this comment

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

same question here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reconcile is used in sfpowerscripts using a dual pass approach, so if we reset the second one will be expensive. Hence made optional, it makes sense though while you retrieve to ensure you ignore any existing cache.

Ideally the fix would be to be have a proper data structure, that caches for every org, as you proposed, but I do think this is a cheaper option for now to get this bug resolved, and then proceed to fix it later

Copy link
Contributor

Choose a reason for hiding this comment

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

For me thats fine fore a temp fix. How do you decide if a cache is valid or not anymore?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
small Pull request is small waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reset the Sfpowerkit on a different target org or include the org name into the cache
3 participants