feat(app-platform): Sort Integrations #12697
Conversation
| renderBody() { | ||
| const {reloading, applications, appInstalls} = this.state; | ||
|
|
||
| const installedProviders = this.providers |
There was a problem hiding this comment.
There's kind of a lot of duplicated and/or very similar code, we could use lodash.partition to cut back some code. Downside of this is that readers will need to look up what partition does. (Current code is fine too though).
const [installedProviders, uninstalledProviders] = _.partition(this.providers.map(p => [p, this.renderProvider(p)]), ([provider,]) => provider.isInstalled);
| // Combine the list of Providers and Sentry Apps that have installations. | ||
| const installed = installedProviders | ||
| .concat(installedSentryApps) | ||
| .sort((a, b) => a[0].localeCompare(b[0])) |
There was a problem hiding this comment.
imo, destructuring adds some more code but can be a bit more clear about what you're retrieving from the array:
const installed = installedProviders
.concat(installedSentryApps)
.sort(([name1,], [name2,]) => name1.localeCompare(name2))
.map([,Component] => Component);
There was a problem hiding this comment.
Yea, more "punctuation", but definitely clearer 👍
|
@billyvg 👍 Sorta tangential, but what's the benefit of those snapshot tests? I don't think I ever really understood. I suppose if you're changing data only, they ensure the rendering is unaffected? Seems like most of the time you're changing rendering and are just going to re-snapshot it, though. |
|
@mnoble when they are 10k lines long... none and I think overall they provide little value. There may be some cases where you're shallow rendering and care about the structure of a component shrug |
|
Heh, fair enough. |
mnoble
force-pushed
the
app-platform/sort-integrations
branch
from
f82437c to
f973812
Compare
|
@mnoble Personally I would trash those jest snapshots and add percy snapshots and an acceptance test. |
"Sentry Apps" and Integrations are now sorted among one another. First by Installed vs Uninstalled status, then alphabetically within each of those groupos.
mnoble
force-pushed
the
app-platform/sort-integrations
branch
from
f973812 to
eff2a4b
Compare
* master: (50 commits) fix(ui) Don't show save-org-search on event search (#12785) ref(ui): Remove some unnecessary index.jsx files (#12606) feat(app-platform): Analytics (#12718) ref(js): Remove ApiMixin (#12384) test(js): Silence project plugin console info spam (#12761) test(js): Move SaveSearchStore.reset() (#12769) test(js): Add more fields to Group fixture (#12759) feat(app-platform): Integration "Learn More" modal (#12638) feat(saved-searches) Move create saved search button to search bar. (#12781) ref(global-header): Remove dead code (#12767) ref(releases): Refactored Releases Serializers (#12535) feat(app-platform): Sort Integrations (#12697) ref(audit-log): Log sso config updates (#12744) ref(app-platform): New 'Open In' UI (#12621) feat(events): Use SnubaEvent if option is turned on (#12594) feat(global-selection-header): show settings icon link in single project mode (#12772) refs(api): Consolidate all search backend code into `SnubaSearchBackend` fix(tests) Remove large snapshots (#12766) fix: Update symbolicator snapshots (#12710) ref: Upgrade semaphore (#12751) ...
"Sentry Apps" and Integrations are now sorted among one another. First by Installed vs Uninstalled status, then alphabetically within each of those groups.