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

fix: rollbar fixes (DEV-1324) #817

Merged
merged 7 commits into from Sep 8, 2022
Merged

fix: rollbar fixes (DEV-1324) #817

merged 7 commits into from Sep 8, 2022

Conversation

mdelez
Copy link
Collaborator

@mdelez mdelez commented Sep 6, 2022

resolves DEV-1324

@mdelez mdelez added the bug A bug fix label Sep 6, 2022
@mdelez mdelez self-assigned this Sep 6, 2022
@mdelez mdelez changed the title fix(project): check if project is undefined before accessing properties (DEV-1324) fix(project): rollbar fixes (DEV-1324) Sep 7, 2022
@mdelez mdelez changed the title fix(project): rollbar fixes (DEV-1324) fix: rollbar fixes (DEV-1324) Sep 7, 2022
@mdelez mdelez marked this pull request as ready for review Sep 8, 2022
@mdelez mdelez requested review from Vijeinath and mpro7 Sep 8, 2022
@mdelez
Copy link
Collaborator Author

mdelez commented Sep 8, 2022

@Vijeinath @mpro7 this isn't really something you guys can test but it just adds a bunch of checks for undefined variables and properties and some silent error handling for failing to retrieve an item from the cache

mpro7
mpro7 approved these changes Sep 8, 2022
Copy link
Collaborator

@mpro7 mpro7 left a comment

Looks good, but check my suggestion.

@@ -63,7 +63,7 @@ export class ResultsComponent {

onSelectionChange(res: FilteredResources) {
this.selectedResources = res;
this.resourceIri = this.selectedResources.resInfo[0].id;
this.resourceIri = this.selectedResources.resInfo[0] ? this.selectedResources.resInfo[0].id : undefined;
Copy link
Collaborator

@mpro7 mpro7 Sep 8, 2022

Choose a reason for hiding this comment

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

Suggested change
this.resourceIri = this.selectedResources.resInfo[0] ? this.selectedResources.resInfo[0].id : undefined;
this.resourceIri = this.selectedResources.resInfo[0]?.id;

Isn't it enough?

Copy link
Collaborator Author

@mdelez mdelez Sep 8, 2022

Choose a reason for hiding this comment

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

you are correct, changed in b53ec4f

@mdelez mdelez merged commit e79eee5 into main Sep 8, 2022
11 checks passed
@mdelez mdelez deleted the wip/dev-1324-rollbar-events branch Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants