Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions DistFiles/localization/en/BloomMediumPriority.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -1384,6 +1384,11 @@
<note>ID: PublishTab.Apps.Build.NeedsPrepareTooltip</note>
<note>Tooltip shown when the user must run Prepare before Build.</note>
</trans-unit>
<trans-unit id="PublishTab.Apps.PlaygroundBookTooltip" translate="no">
<source xml:lang="en">Books made from the Playground template cannot be published: %0</source>
<note>ID: PublishTab.Apps.PlaygroundBookTooltip</note>
<note>Tooltip on the disabled Prepare and Build buttons in Publish > Apps when a book chosen for the app was made from the Playground template. "Playground" is the name of a Bloom template book. %0 is replaced with the title(s) of the offending book(s).</note>
</trans-unit>
<trans-unit id="PublishTab.Apps.Build.Tooltip">
<source xml:lang="en">Build a new APK from the current settings and selected books.</source>
<note>ID: PublishTab.Apps.Build.Tooltip</note>
Expand Down
43 changes: 33 additions & 10 deletions src/BloomBrowserUI/publish/Apps/AppPublisherScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,16 @@ const AppPublisherScreenContents: React.FunctionComponent<{
"Run Prepare before building the app.",
"PublishTab.Apps.Build.NeedsPrepareTooltip",
);
// Like every other publish screen, refuse to publish a Playground book (BL-16855). Here the
// check covers every book headed into the app, not just the current one.
const playgroundBookTitles = screenState.status.playgroundBookTitles;
const isPlaygroundBook = playgroundBookTitles.length > 0;
const playgroundBookTooltip = useL10n(
"Books made from the Playground template cannot be published: %0",
"PublishTab.Apps.PlaygroundBookTooltip",
"%0 is replaced with the title(s) of the offending book(s).",
playgroundBookTitles.join(", "),
);
const tryOnPhoneTooltip = useL10n(
"Load and run the app on your phone. First enable USB Debugging on the phone and connect it with a USB cable.",
"PublishTab.Apps.TryOnPhone.Tooltip",
Expand Down Expand Up @@ -242,9 +252,12 @@ const AppPublisherScreenContents: React.FunctionComponent<{
const buildIsNeeded = screenState.buildIsNeeded;
const busyAction = screenState.busyAction;
const apkIsCurrent = screenState.status.apkExists && !buildIsNeeded;
const canRunPrepare = !busyAction && !prepareIsReady;
const canRunPrepare = !busyAction && !prepareIsReady && !isPlaygroundBook;
const canUseConfiguredProject = prepareIsReady && !busyAction;
const canRunBuild = !busyAction && screenState.hasRequiredBuildSettings;
const canRunBuild =
!busyAction &&
screenState.hasRequiredBuildSettings &&
!isPlaygroundBook;
const canUseCurrentApk = apkIsCurrent && !busyAction;
const activePrepareStepId = getPrepareStepIdForStage(
busyAction,
Expand Down Expand Up @@ -294,14 +307,24 @@ const AppPublisherScreenContents: React.FunctionComponent<{
complete: progressCompleteLabel,
},
);
const buildTooltipToShow = !prepareIsReady
? buildNeedsPrepareTooltip
: buildIsNeeded
? buildTooltip
: buildDoneTooltip;
const prepareTooltipToShow = prepareIsReady
? prepareDoneTooltip
: prepareTooltip;
let buildTooltipToShow: string;
if (isPlaygroundBook) {
buildTooltipToShow = playgroundBookTooltip;
} else if (!prepareIsReady) {
buildTooltipToShow = buildNeedsPrepareTooltip;
} else if (buildIsNeeded) {
buildTooltipToShow = buildTooltip;
} else {
buildTooltipToShow = buildDoneTooltip;
}
let prepareTooltipToShow: string;
if (isPlaygroundBook) {
prepareTooltipToShow = playgroundBookTooltip;
} else if (prepareIsReady) {
prepareTooltipToShow = prepareDoneTooltip;
} else {
prepareTooltipToShow = prepareTooltip;
}
const validationIssueLabels: string[] = [];
if (screenState.settingsValidationIssues.appName) {
validationIssueLabels.push(appNameLabel);
Expand Down
8 changes: 8 additions & 0 deletions src/BloomBrowserUI/publish/Apps/appBuilderShared.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ describe("appBuilderShared prepare steps", () => {
]);
});

it("normalizes playground book titles and defaults them to empty", () => {
expect(
normalizeStatus({ PlaygroundBookTitles: ["My Playground"] })
.playgroundBookTitles,
).toEqual(["My Playground"]);
expect(normalizeStatus({}).playgroundBookTitles).toEqual([]);
});

it("maps prepare websocket stages onto prepare step ids", () => {
expect(getPrepareStepIdForStage("prepare", "running-installer")).toBe(
"rab-installed",
Expand Down
7 changes: 7 additions & 0 deletions src/BloomBrowserUI/publish/Apps/appBuilderShared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ export interface IAppBuilderStatus {
apkSizeBytes?: number;
rabRoot?: string;
trackedBookTitles?: string[];
/** Titles of tracked books made from the Playground template; such books cannot be published. */
playgroundBookTitles: string[];
trackedBooks: IAppBuilderTrackedBook[];
/** The action currently running on the server, or undefined if idle. */
activeAction?: AppBuilderAction;
Expand Down Expand Up @@ -80,6 +82,7 @@ export interface IAppBuilderStatusApi {
apkSizeBytes?: number;
rabRoot?: string;
trackedBookTitles?: string[];
playgroundBookTitles?: string[];
activeAction?: string;
activeActionProgressStage?: string;
activeActionProgressPercent?: number;
Expand All @@ -98,6 +101,7 @@ export interface IAppBuilderStatusApi {
ApkSizeBytes?: number;
RabRoot?: string;
TrackedBookTitles?: string[];
PlaygroundBookTitles?: string[];
trackedBooks?: IAppBuilderTrackedBookApi[];
TrackedBooks?: IAppBuilderTrackedBookApi[];
}
Expand Down Expand Up @@ -154,6 +158,7 @@ export const defaultStatus: IAppBuilderStatus = {
buildNeeded: false,
prepareSteps: getDefaultPrepareSteps(),
trackedBookTitles: [],
playgroundBookTitles: [],
trackedBooks: [],
};

Expand Down Expand Up @@ -253,6 +258,8 @@ export function normalizeStatus(
rabRoot: status?.rabRoot ?? status?.RabRoot,
trackedBookTitles:
status?.trackedBookTitles ?? status?.TrackedBookTitles ?? [],
playgroundBookTitles:
status?.playgroundBookTitles ?? status?.PlaygroundBookTitles ?? [],
trackedBooks: (status?.trackedBooks ?? status?.TrackedBooks ?? []).map(
normalizeTrackedBook,
),
Expand Down
9 changes: 1 addition & 8 deletions src/BloomExe/Book/Book.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6209,13 +6209,6 @@ public string GetDefaultTemplatePageId()
return Storage.Dom.GetMetaValue("defaultTemplatePageId", null);
}

public bool IsPlayground
{
get
{
return BookInfo.BookLineage?.Contains("aeb176bc-76fa-44e2-bb9d-6350698fce47")
?? false;
}
}
public bool IsPlayground => BookInfo.IsPlayground;
}
}
12 changes: 12 additions & 0 deletions src/BloomExe/Book/BookInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,18 @@ public string BookLineage
set { MetaData.BookLineage = value; }
}

/// <summary>
/// The id of the Playground template book. Books derived from it get every feature
/// unlocked for experimenting, and in exchange can never be published.
/// </summary>
public const string kPlaygroundTemplateId = "aeb176bc-76fa-44e2-bb9d-6350698fce47";

/// <summary>
/// True if this book was made from the Playground template (see kPlaygroundTemplateId).
/// Cheap: it only reads the lineage from meta.json, so it needs no Book object.
/// </summary>
public bool IsPlayground => BookLineage?.Contains(kPlaygroundTemplateId) ?? false;

// This indicates the kind of license in use. For Creative Commons licenses, it is the Abbreviation of the CreativeCommonsLicense
// object, the second-last (before version number) element of the licenseUrl. Other known values are 'ask' (no license granted,
// ask the copyright holder for permission to use) 'custom' (rights presumably specified in licenseNotes)
Expand Down
6 changes: 6 additions & 0 deletions src/BloomExe/Publish/Rab/RabProjectModels.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ public class RabProjectStatus
public long ApkSizeBytes { get; set; }
public string RabRoot { get; set; }
public string[] TrackedBookTitles { get; set; } = Array.Empty<string>();

/// <summary>
/// Titles of the books headed into the app that were made from the Playground template.
/// Such books can never be published, so the UI blocks Prepare/Build while this is non-empty.
/// </summary>
public string[] PlaygroundBookTitles { get; set; } = Array.Empty<string>();
public RabTrackedBookInfo[] TrackedBooks { get; set; } = Array.Empty<RabTrackedBookInfo>();
public RabPrepareStepStatus[] PrepareSteps { get; set; } =
Array.Empty<RabPrepareStepStatus>();
Expand Down
70 changes: 63 additions & 7 deletions src/BloomExe/Publish/Rab/RabProjectService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,11 @@ public RabProjectStatus GetStatus()
RabRoot = paths.RabRoot,
TrackedBooks = trackedBooks,
TrackedBookTitles = trackedBooks.Select(book => book.Title).ToArray(),
// The Apps screen disables Prepare and Build while any of these exist (BL-16855).
PlaygroundBookTitles = trackedBooks
.Where(book => TryFindTrackedBookInfo(book)?.IsPlayground ?? false)
.Select(book => book.Title)
.ToArray(),
PrepareSteps = prepareSteps,
ActiveAction = activeAction,
ActiveActionProgressStage = activeAction != null ? _lastLoggedProgressStage : null,
Expand Down Expand Up @@ -1415,6 +1420,14 @@ private List<RabBookPublishInfo> ExportBookInfos(
var booksToExport = bookInfos.ToList();
var bloomPubPathsToKeep = new HashSet<string>(StringComparer.OrdinalIgnoreCase);

// Playground books are never publishable (BL-16855). The Apps screen disables Prepare and
// Build for a Playground current book, but Choose Books can add one from the collection.
EnsureNoPlaygroundBooks(
booksToExport,
bookInfo =>
GetBookTitleForRab(_collectionModel.GetBookFromBookInfo(bookInfo), bookInfo)
);

// Like the other publish paths, refuse to publish a book in a language its copyright holder
// has not licensed (BL-16833). Check every book, including ones whose BloomPUB we would
// merely reuse, before touching anything on disk.
Expand Down Expand Up @@ -1522,6 +1535,31 @@ private List<RabBookPublishInfo> ExportBookInfos(
return exportedBooks;
}

/// <summary>
/// Stops Prepare/Build when any book headed into the app was made from the Playground template,
/// which (like every other publish path) we refuse to publish. Throws naming each such book.
/// </summary>
/// <remarks>
/// Only the offending books have their title looked up, so the common case costs no Book load.
/// </remarks>
internal static void EnsureNoPlaygroundBooks(
IEnumerable<BookInfo> bookInfos,
Func<BookInfo, string> getTitle
)
{
var playgroundTitles = bookInfos
.Where(bookInfo => bookInfo.IsPlayground)
.Select(getTitle)
.ToList();
if (playgroundTitles.Count == 0)
return;

throw new ApplicationException(
"Books made from the Playground template cannot be published: "
+ string.Join(", ", playgroundTitles)
);
}

/// <summary>
/// Stops Prepare/Build when any book headed into the app may not be published in the languages its
/// BloomPUB would include (see LicenseChecker), by throwing with the LicenseChecker message; the
Expand Down Expand Up @@ -3613,8 +3651,33 @@ private static string GetExistingThumbnailFileName(RabBookPublishInfo book)
return book?.ThumbnailFileName;
}

/// <summary>
/// Finds the collection book a tracked entry refers to, throwing if it is no longer there.
/// Use this on the Prepare/Build paths, where a missing book must fail the action.
/// </summary>
private BookInfo FindTrackedBookInfo(RabTrackedBookInfo trackedBook)
{
var matchingBookInfo = TryFindTrackedBookInfo(trackedBook);
if (matchingBookInfo == null)
{
throw new ApplicationException(
$"Bloom could not find the selected book '{trackedBook.Title}' in this collection anymore."
);
}

return matchingBookInfo;
}

/// <summary>
/// Like FindTrackedBookInfo, but returns null when the book is no longer in the collection.
/// GetStatus uses this so a stale tracked entry cannot stop the Apps screen from loading.
/// </summary>
private BookInfo TryFindTrackedBookInfo(RabTrackedBookInfo trackedBook)
{
// Unit tests, and Bloom before a collection is loaded, have no collection model.
if (_collectionModel?.TheOneEditableCollection == null)
return null;

var bookInfos = _collectionModel.TheOneEditableCollection.GetBookInfos();
var matchingBookInfo = !string.IsNullOrWhiteSpace(trackedBook.BookId)
? bookInfos.FirstOrDefault(info =>
Expand Down Expand Up @@ -3674,13 +3737,6 @@ private BookInfo FindTrackedBookInfo(RabTrackedBookInfo trackedBook)
matchingBookInfo = titleMatches[0];
}

if (matchingBookInfo == null)
{
throw new ApplicationException(
$"Bloom could not find the selected book '{trackedBook.Title}' in this collection anymore."
);
}

return matchingBookInfo;
}

Expand Down
57 changes: 57 additions & 0 deletions src/BloomTests/Publish/Rab/RabPlaygroundCheckTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
using System;
using Bloom.Book;
using Bloom.Publish.Rab;
using BloomTests.Book;
using NUnit.Framework;

namespace BloomTests.Publish.Rab
{
/// <summary>
/// Publish > Apps must refuse to put a book made from the Playground template into an app (BL-16855),
/// as every other publish path refuses to publish such a book.
/// </summary>
[TestFixture]
public class RabPlaygroundCheckTests : BookTestsBase
{
private const string kPlaygroundTemplateId = BookInfo.kPlaygroundTemplateId;

protected override string GetTestFolderName() => "RabPlaygroundCheckTests";

[Test]
public void EnsureNoPlaygroundBooks_PlaygroundBook_ThrowsNamingBook()
{
var book = CreateBook();
book.BookInfo.BookLineage = kPlaygroundTemplateId;
// Sanity check that the lineage really makes this a Playground book.
Assert.That(book.IsPlayground, Is.True);

var exception = Assert.Throws<ApplicationException>(() =>
RabProjectService.EnsureNoPlaygroundBooks(
new[] { book.BookInfo },
bookInfo => "My Playground"
)
);

Assert.That(exception.Message, Does.Contain("Playground template"));
Assert.That(exception.Message, Does.Contain("My Playground"));
}

[Test]
public void EnsureNoPlaygroundBooks_OrdinaryBook_DoesNotThrow()
{
var book = CreateBook();
Assert.That(book.IsPlayground, Is.False);

Assert.DoesNotThrow(() =>
RabProjectService.EnsureNoPlaygroundBooks(
new[] { book.BookInfo },
bookInfo =>
{
Assert.Fail("Titles should only be looked up for Playground books.");
return "";
}
)
);
}
}
}