Allow selecting a locked SD card as the source drive

Changelog-entry: Allow selecting a locked SD card as the source drive
Change-type: patch
This commit is contained in:
Alexis Svinartchouk 2020-11-16 14:16:38 +01:00
parent 1e0a6a3129
commit e7b4f09021
8 changed files with 73 additions and 68 deletions

View File

@ -138,6 +138,7 @@ const InitProgress = styled(
export interface DriveSelectorProps export interface DriveSelectorProps
extends Omit<ModalProps, 'done' | 'cancel'> { extends Omit<ModalProps, 'done' | 'cancel'> {
write: boolean;
multipleSelection: boolean; multipleSelection: boolean;
showWarnings?: boolean; showWarnings?: boolean;
cancel: () => void; cancel: () => void;
@ -258,7 +259,8 @@ export class DriveSelector extends React.Component<
return ( return (
isUsbbootDrive(drive) || isUsbbootDrive(drive) ||
isDriverlessDrive(drive) || isDriverlessDrive(drive) ||
!isDriveValid(drive, image) !isDriveValid(drive, image) ||
(this.props.write && drive.isReadOnly)
); );
} }
@ -311,6 +313,7 @@ export class DriveSelector extends React.Component<
const statuses: DriveStatus[] = getDriveImageCompatibilityStatuses( const statuses: DriveStatus[] = getDriveImageCompatibilityStatuses(
drive, drive,
this.state.image, this.state.image,
this.props.write,
).slice(0, 2); ).slice(0, 2);
return ( return (
// the column render fn expects a single Element // the column render fn expects a single Element

View File

@ -715,6 +715,7 @@ export class SourceSelector extends React.Component<
{showDriveSelector && ( {showDriveSelector && (
<DriveSelector <DriveSelector
write={false}
multipleSelection={false} multipleSelection={false}
titleLabel="Select source" titleLabel="Select source"
emptyListLabel="Plug a source" emptyListLabel="Plug a source"

View File

@ -24,7 +24,7 @@ import {
} from '../../../../shared/drive-constraints'; } from '../../../../shared/drive-constraints';
import { compatibility, warning } from '../../../../shared/messages'; import { compatibility, warning } from '../../../../shared/messages';
import * as prettyBytes from 'pretty-bytes'; import * as prettyBytes from 'pretty-bytes';
import { getSelectedDrives } from '../../models/selection-state'; import { getImage, getSelectedDrives } from '../../models/selection-state';
import { import {
ChangeButton, ChangeButton,
DetailsText, DetailsText,
@ -80,9 +80,11 @@ export function TargetSelectorButton(props: TargetSelectorProps) {
if (targets.length === 1) { if (targets.length === 1) {
const target = targets[0]; const target = targets[0];
const warnings = getDriveImageCompatibilityStatuses(target).map( const warnings = getDriveImageCompatibilityStatuses(
getDriveWarning, target,
); getImage(),
true,
).map(getDriveWarning);
return ( return (
<> <>
<StepNameButton plain tooltip={props.tooltip}> <StepNameButton plain tooltip={props.tooltip}>
@ -106,9 +108,11 @@ export function TargetSelectorButton(props: TargetSelectorProps) {
if (targets.length > 1) { if (targets.length > 1) {
const targetsTemplate = []; const targetsTemplate = [];
for (const target of targets) { for (const target of targets) {
const warnings = getDriveImageCompatibilityStatuses(target).map( const warnings = getDriveImageCompatibilityStatuses(
getDriveWarning, target,
); getImage(),
true,
).map(getDriveWarning);
targetsTemplate.push( targetsTemplate.push(
<DetailsText <DetailsText
key={target.device} key={target.device}

View File

@ -161,6 +161,7 @@ export const TargetSelector = ({
{showTargetSelectorModal && ( {showTargetSelectorModal && (
<TargetSelectorModal <TargetSelectorModal
write={true}
cancel={() => setShowTargetSelectorModal(false)} cancel={() => setShowTargetSelectorModal(false)}
done={(modalTargets) => { done={(modalTargets) => {
selectAllTargets(modalTargets); selectAllTargets(modalTargets);

View File

@ -198,6 +198,7 @@ function storeReducer(
(accState, drive) => { (accState, drive) => {
if ( if (
constraints.isDriveValid(drive, image) && constraints.isDriveValid(drive, image) &&
!drive.isReadOnly &&
constraints.isDriveSizeRecommended(drive, image) && constraints.isDriveSizeRecommended(drive, image) &&
// We don't want to auto-select large drives execpt is autoSelectAllDrives is true // We don't want to auto-select large drives execpt is autoSelectAllDrives is true
(!constraints.isDriveSizeLarge(drive) || shouldAutoselectAll) && (!constraints.isDriveSizeLarge(drive) || shouldAutoselectAll) &&

View File

@ -199,7 +199,11 @@ export class FlashStep extends React.PureComponent<
const drives = selection.getSelectedDrives().map((drive) => { const drives = selection.getSelectedDrives().map((drive) => {
return { return {
...drive, ...drive,
statuses: constraints.getDriveImageCompatibilityStatuses(drive), statuses: constraints.getDriveImageCompatibilityStatuses(
drive,
undefined,
true,
),
}; };
}); });
if (drives.length === 0 || this.props.isFlashing) { if (drives.length === 0 || this.props.isFlashing) {
@ -308,6 +312,7 @@ export class FlashStep extends React.PureComponent<
)} )}
{this.state.showDriveSelectorModal && ( {this.state.showDriveSelectorModal && (
<TargetSelectorModal <TargetSelectorModal
write={true}
cancel={() => this.setState({ showDriveSelectorModal: false })} cancel={() => this.setState({ showDriveSelectorModal: false })}
done={(modalTargets) => { done={(modalTargets) => {
selectAllTargets(modalTargets); selectAllTargets(modalTargets);

View File

@ -34,16 +34,6 @@ export type DrivelistDrive = Drive & {
displayName: string; displayName: string;
}; };
/**
* @summary Check if a drive is locked
*
* @description
* This usually points out a locked SD Card.
*/
export function isDriveLocked(drive: DrivelistDrive): boolean {
return Boolean(drive.isReadOnly);
}
/** /**
* @summary Check if a drive is a system drive * @summary Check if a drive is a system drive
*/ */
@ -122,14 +112,13 @@ export function isDriveDisabled(drive: DrivelistDrive): boolean {
} }
/** /**
* @summary Check if a drive is valid, i.e. not locked and large enough for an image * @summary Check if a drive is valid, i.e. large enough for an image
*/ */
export function isDriveValid( export function isDriveValid(
drive: DrivelistDrive, drive: DrivelistDrive,
image?: SourceMetadata, image?: SourceMetadata,
): boolean { ): boolean {
return ( return (
!isDriveLocked(drive) &&
isDriveLargeEnough(drive, image) && isDriveLargeEnough(drive, image) &&
!isSourceDrive(drive, image as SourceMetadata) && !isSourceDrive(drive, image as SourceMetadata) &&
!isDriveDisabled(drive) !isDriveDisabled(drive)
@ -213,17 +202,19 @@ export const statuses = {
*/ */
export function getDriveImageCompatibilityStatuses( export function getDriveImageCompatibilityStatuses(
drive: DrivelistDrive, drive: DrivelistDrive,
image?: SourceMetadata, image: SourceMetadata | undefined,
write: boolean,
) { ) {
const statusList = []; const statusList = [];
// Mind the order of the if-statements if you modify. // Mind the order of the if-statements if you modify.
if (isDriveLocked(drive)) { if (drive.isReadOnly && write) {
statusList.push({ statusList.push({
type: COMPATIBILITY_STATUS_TYPES.ERROR, type: COMPATIBILITY_STATUS_TYPES.ERROR,
message: messages.compatibility.locked(), message: messages.compatibility.locked(),
}); });
} else if ( }
if (
!_.isNil(drive) && !_.isNil(drive) &&
!_.isNil(drive.size) && !_.isNil(drive.size) &&
!isDriveLargeEnough(drive, image) !isDriveLargeEnough(drive, image)
@ -262,10 +253,11 @@ export function getDriveImageCompatibilityStatuses(
*/ */
export function getListDriveImageCompatibilityStatuses( export function getListDriveImageCompatibilityStatuses(
drives: DrivelistDrive[], drives: DrivelistDrive[],
image: SourceMetadata, image: SourceMetadata | undefined,
write: boolean,
) { ) {
return drives.flatMap((drive) => { return drives.flatMap((drive) => {
return getDriveImageCompatibilityStatuses(drive, image); return getDriveImageCompatibilityStatuses(drive, image, write);
}); });
} }
@ -277,9 +269,12 @@ export function getListDriveImageCompatibilityStatuses(
*/ */
export function hasDriveImageCompatibilityStatus( export function hasDriveImageCompatibilityStatus(
drive: DrivelistDrive, drive: DrivelistDrive,
image: SourceMetadata, image: SourceMetadata | undefined,
write: boolean,
) { ) {
return Boolean(getDriveImageCompatibilityStatuses(drive, image).length); return Boolean(
getDriveImageCompatibilityStatuses(drive, image, write).length,
);
} }
export interface DriveStatus { export interface DriveStatus {

View File

@ -23,37 +23,6 @@ import * as constraints from '../../lib/shared/drive-constraints';
import * as messages from '../../lib/shared/messages'; import * as messages from '../../lib/shared/messages';
describe('Shared: DriveConstraints', function () { describe('Shared: DriveConstraints', function () {
describe('.isDriveLocked()', function () {
it('should return true if the drive is read-only', function () {
const result = constraints.isDriveLocked({
device: '/dev/disk2',
size: 999999999,
isReadOnly: true,
} as constraints.DrivelistDrive);
expect(result).to.be.true;
});
it('should return false if the drive is not read-only', function () {
const result = constraints.isDriveLocked({
device: '/dev/disk2',
size: 999999999,
isReadOnly: false,
} as constraints.DrivelistDrive);
expect(result).to.be.false;
});
it("should return false if we don't know if the drive is read-only", function () {
const result = constraints.isDriveLocked({
device: '/dev/disk2',
size: 999999999,
} as constraints.DrivelistDrive);
expect(result).to.be.false;
});
});
describe('.isSystemDrive()', function () { describe('.isSystemDrive()', function () {
it('should return true if the drive is a system drive', function () { it('should return true if the drive is a system drive', function () {
const result = constraints.isSystemDrive({ const result = constraints.isSystemDrive({
@ -745,7 +714,7 @@ describe('Shared: DriveConstraints', function () {
this.drive.disabled = false; this.drive.disabled = false;
}); });
it('should return false if the drive is not large enough and is a source drive', function () { it('should return false if the drive is not large enough and is the source drive', function () {
expect( expect(
constraints.isDriveValid(this.drive, { constraints.isDriveValid(this.drive, {
...image, ...image,
@ -755,7 +724,7 @@ describe('Shared: DriveConstraints', function () {
).to.be.false; ).to.be.false;
}); });
it('should return false if the drive is not large enough and is not a source drive', function () { it('should return false if the drive is not large enough and is not the source drive', function () {
expect( expect(
constraints.isDriveValid(this.drive, { constraints.isDriveValid(this.drive, {
...image, ...image,
@ -765,17 +734,17 @@ describe('Shared: DriveConstraints', function () {
).to.be.false; ).to.be.false;
}); });
it('should return false if the drive is large enough and is a source drive', function () { it('should return true if the drive is large enough and is the source drive', function () {
expect(constraints.isDriveValid(this.drive, image)).to.be.false; expect(constraints.isDriveValid(this.drive, image)).to.be.true;
}); });
it('should return false if the drive is large enough and is not a source drive', function () { it('should return true if the drive is large enough and is not the source drive', function () {
expect( expect(
constraints.isDriveValid(this.drive, { constraints.isDriveValid(this.drive, {
...image, ...image,
path: path.resolve(this.mountpoint, '../bar/rpi.img'), path: path.resolve(this.mountpoint, '../bar/rpi.img'),
}), }),
).to.be.false; ).to.be.true;
}); });
}); });
}); });
@ -983,6 +952,7 @@ describe('Shared: DriveConstraints', function () {
const result = constraints.getDriveImageCompatibilityStatuses( const result = constraints.getDriveImageCompatibilityStatuses(
this.drive, this.drive,
this.image, this.image,
true,
); );
expect(result).to.deep.equal([]); expect(result).to.deep.equal([]);
@ -995,6 +965,7 @@ describe('Shared: DriveConstraints', function () {
const result = constraints.getDriveImageCompatibilityStatuses( const result = constraints.getDriveImageCompatibilityStatuses(
this.drive, this.drive,
this.image, this.image,
true,
); );
const expectedTuples: Array<['WARNING' | 'ERROR', string]> = []; const expectedTuples: Array<['WARNING' | 'ERROR', string]> = [];
@ -1009,6 +980,7 @@ describe('Shared: DriveConstraints', function () {
const result = constraints.getDriveImageCompatibilityStatuses( const result = constraints.getDriveImageCompatibilityStatuses(
this.drive, this.drive,
this.image, this.image,
true,
); );
// @ts-ignore // @ts-ignore
const expectedTuples = [['ERROR', 'containsImage']]; const expectedTuples = [['ERROR', 'containsImage']];
@ -1025,6 +997,7 @@ describe('Shared: DriveConstraints', function () {
const result = constraints.getDriveImageCompatibilityStatuses( const result = constraints.getDriveImageCompatibilityStatuses(
this.drive, this.drive,
this.image, this.image,
true,
); );
const expectedTuples = [['WARNING', 'system']]; const expectedTuples = [['WARNING', 'system']];
@ -1040,6 +1013,7 @@ describe('Shared: DriveConstraints', function () {
const result = constraints.getDriveImageCompatibilityStatuses( const result = constraints.getDriveImageCompatibilityStatuses(
this.drive, this.drive,
this.image, this.image,
true,
); );
const expected = [ const expected = [
{ {
@ -1060,6 +1034,7 @@ describe('Shared: DriveConstraints', function () {
const result = constraints.getDriveImageCompatibilityStatuses( const result = constraints.getDriveImageCompatibilityStatuses(
this.drive, this.drive,
this.image, this.image,
true,
); );
// @ts-ignore // @ts-ignore
const expectedTuples = []; const expectedTuples = [];
@ -1076,6 +1051,7 @@ describe('Shared: DriveConstraints', function () {
const result = constraints.getDriveImageCompatibilityStatuses( const result = constraints.getDriveImageCompatibilityStatuses(
this.drive, this.drive,
this.image, this.image,
true,
); );
// @ts-ignore // @ts-ignore
const expectedTuples = [['ERROR', 'locked']]; const expectedTuples = [['ERROR', 'locked']];
@ -1092,6 +1068,7 @@ describe('Shared: DriveConstraints', function () {
const result = constraints.getDriveImageCompatibilityStatuses( const result = constraints.getDriveImageCompatibilityStatuses(
this.drive, this.drive,
this.image, this.image,
true,
); );
// @ts-ignore // @ts-ignore
const expectedTuples = [['WARNING', 'sizeNotRecommended']]; const expectedTuples = [['WARNING', 'sizeNotRecommended']];
@ -1108,6 +1085,7 @@ describe('Shared: DriveConstraints', function () {
const result = constraints.getDriveImageCompatibilityStatuses( const result = constraints.getDriveImageCompatibilityStatuses(
this.drive, this.drive,
this.image, this.image,
true,
); );
const expectedTuples = [['WARNING', 'largeDrive']]; const expectedTuples = [['WARNING', 'largeDrive']];
@ -1128,9 +1106,13 @@ describe('Shared: DriveConstraints', function () {
const result = constraints.getDriveImageCompatibilityStatuses( const result = constraints.getDriveImageCompatibilityStatuses(
this.drive, this.drive,
this.image, this.image,
true,
); );
// @ts-ignore // @ts-ignore
const expectedTuples = [['ERROR', 'locked']]; const expectedTuples = [
['ERROR', 'locked'],
['ERROR', 'containsImage'],
];
// @ts-ignore // @ts-ignore
expectStatusTypesAndMessagesToBe(result, expectedTuples); expectStatusTypesAndMessagesToBe(result, expectedTuples);
@ -1144,6 +1126,7 @@ describe('Shared: DriveConstraints', function () {
const result = constraints.getDriveImageCompatibilityStatuses( const result = constraints.getDriveImageCompatibilityStatuses(
this.drive, this.drive,
this.image, this.image,
true,
); );
// @ts-ignore // @ts-ignore
const expectedTuples = [['ERROR', 'locked']]; const expectedTuples = [['ERROR', 'locked']];
@ -1161,6 +1144,7 @@ describe('Shared: DriveConstraints', function () {
const result = constraints.getDriveImageCompatibilityStatuses( const result = constraints.getDriveImageCompatibilityStatuses(
this.drive, this.drive,
this.image, this.image,
true,
); );
const expected = [ const expected = [
{ {
@ -1181,6 +1165,7 @@ describe('Shared: DriveConstraints', function () {
const result = constraints.getDriveImageCompatibilityStatuses( const result = constraints.getDriveImageCompatibilityStatuses(
this.drive, this.drive,
this.image, this.image,
true,
); );
// @ts-ignore // @ts-ignore
const expectedTuples = [ const expectedTuples = [
@ -1287,7 +1272,7 @@ describe('Shared: DriveConstraints', function () {
describe('given no drives', function () { describe('given no drives', function () {
it('should return no statuses', function () { it('should return no statuses', function () {
expect( expect(
constraints.getListDriveImageCompatibilityStatuses([], image), constraints.getListDriveImageCompatibilityStatuses([], image, true),
).to.deep.equal([]); ).to.deep.equal([]);
}); });
}); });
@ -1298,6 +1283,7 @@ describe('Shared: DriveConstraints', function () {
constraints.getListDriveImageCompatibilityStatuses( constraints.getListDriveImageCompatibilityStatuses(
[drives[0]], [drives[0]],
image, image,
true,
), ),
).to.deep.equal([ ).to.deep.equal([
{ {
@ -1312,6 +1298,7 @@ describe('Shared: DriveConstraints', function () {
constraints.getListDriveImageCompatibilityStatuses( constraints.getListDriveImageCompatibilityStatuses(
[drives[1]], [drives[1]],
image, image,
true,
), ),
).to.deep.equal([ ).to.deep.equal([
{ {
@ -1326,6 +1313,7 @@ describe('Shared: DriveConstraints', function () {
constraints.getListDriveImageCompatibilityStatuses( constraints.getListDriveImageCompatibilityStatuses(
[drives[2]], [drives[2]],
image, image,
true,
), ),
).to.deep.equal([ ).to.deep.equal([
{ {
@ -1340,6 +1328,7 @@ describe('Shared: DriveConstraints', function () {
constraints.getListDriveImageCompatibilityStatuses( constraints.getListDriveImageCompatibilityStatuses(
[drives[3]], [drives[3]],
image, image,
true,
), ),
).to.deep.equal([ ).to.deep.equal([
{ {
@ -1354,6 +1343,7 @@ describe('Shared: DriveConstraints', function () {
constraints.getListDriveImageCompatibilityStatuses( constraints.getListDriveImageCompatibilityStatuses(
[drives[4]], [drives[4]],
image, image,
true,
), ),
).to.deep.equal([ ).to.deep.equal([
{ {
@ -1368,6 +1358,7 @@ describe('Shared: DriveConstraints', function () {
constraints.getListDriveImageCompatibilityStatuses( constraints.getListDriveImageCompatibilityStatuses(
[drives[5]], [drives[5]],
image, image,
true,
), ),
).to.deep.equal([ ).to.deep.equal([
{ {
@ -1381,7 +1372,11 @@ describe('Shared: DriveConstraints', function () {
describe('given multiple drives with all warnings/errors', function () { describe('given multiple drives with all warnings/errors', function () {
it('should return all statuses', function () { it('should return all statuses', function () {
expect( expect(
constraints.getListDriveImageCompatibilityStatuses(drives, image), constraints.getListDriveImageCompatibilityStatuses(
drives,
image,
true,
),
).to.deep.equal([ ).to.deep.equal([
{ {
message: 'Source drive', message: 'Source drive',