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.

Sign up for GitHub

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

Jump to bottom

Workaround for legacy added games to have the ability to be deleted #6019

Merged
merged 5 commits into from
Aug 17, 2023
Merged

Workaround for legacy added games to have the ability to be deleted #6019

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b8160d3
Workaround for legacy added games to have the ability to be deleted
srietkerk Aug 11, 2023
053b568
Merge branch 'master' of https://github.com/microsoft/pxt-arcade into…
srietkerk Aug 13, 2023
9b673d1
Remove optional chaining from game deleted check
srietkerk Aug 14, 2023
66235b4
Null check added when populating games to kiosk
srietkerk Aug 13, 2023
a74ddc2
Kiosk: New build for lock fixes, deleting legacy added games
srietkerk Aug 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions kiosk/src/Models/Kiosk.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@ export class Kiosk {
const addedGames = this.getAllAddedGames();
const addedGamesObjs: GameData[] = Object.values(addedGames);
for (const game of addedGamesObjs) {
if (!game?.userAdded) {
game.userAdded = true;
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (!game?.userAdded) {
game.userAdded = true;
}
game.userAdded = true;

should be fine to just mark all the games? fwiw this one caught my eye because the ?. on the previous line wouldn't actually protect us (as it would still get a null pointer exception on line 141 if game == null, and similar on the one a few lines down (that won't get an npe, but would probably get some weird state if game == null because this.games would end up with a nullish entry in it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we just have a null check at the start of the inside of this for loop? It would certainly give some peace of mind just to make sure that if we have a nullish game, it won't be added to the list. I'm not sure if that's possible and it thus might be adding a branch for no reason, but it might just be a protection that we want to have.

if (!game?.deleted) {
srietkerk marked this conversation as resolved.
Show resolved Hide resolved
this.games.push(game);
}
Expand Down
Loading