Skip to content

feat: Allow user to control message view mode #10945

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: Allow user to control message view mode #10945

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2c6772c
feat: Allow disabling of threaded view
SebastianKrupinski Mar 29, 2025
3d6cd5e
fixup! feat: Allow disabling of threaded view
SebastianKrupinski Apr 5, 2025
8498a7a
fixup! feat: Allow disabling of threaded view
SebastianKrupinski Apr 5, 2025
0e91868
fixup! feat: Allow disabling of threaded view
SebastianKrupinski Apr 7, 2025
63b7546
fixup! feat: Allow disabling of threaded view
SebastianKrupinski May 2, 2025
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 appinfo/routes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,11 @@
'url' => '/api/settings/importance-classification-default',
'verb' => 'PUT'
],
[
'name' => 'settings#setLayoutMessageView',
'url' => '/api/settings/layout-message-view',
'verb' => 'PUT'
],
[
'name' => 'trusted_senders#setTrusted',
'url' => '/api/trustedsenders/{email}',
Expand Down
6 changes: 5 additions & 1 deletion lib/Contracts/IMailSearch.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
interface IMailSearch {
public const ORDER_NEWEST_FIRST = 'DESC';
public const ORDER_OLDEST_FIRST = 'ASC';
public const VIEW_SINGLETON = 'singleton';
public const VIEW_THREADED = 'threaded';
/**
* @throws DoesNotExistException
* @throws ClientException
Expand All @@ -36,6 +38,7 @@ public function findMessage(Account $account,
* @param string|null $filter
* @param int|null $cursor
* @param int|null $limit
* @param string|null $view
*
* @return Message[]
*
Expand All @@ -47,7 +50,8 @@ public function findMessages(Account $account,
string $sortOrder,
?string $filter,
?int $cursor,
?int $limit): array;
?int $limit,
?string $view): array;

/**
* @param IUser $user
Expand Down
34 changes: 23 additions & 11 deletions lib/Controller/MessagesController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@
* @param int $cursor
* @param string $filter
* @param int|null $limit
* @param string $sort returns messages in requested order ('newest' or 'oldest')
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
* @param string $sort returns messages in requested order ('newest' or 'oldest')
* @param IMailSearcH::sort_* $sort

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't use IMailSearcH::sort_* here the UI sends 'newest' or 'oldest' and the value of IMailSearch::ORDER_* is 'DESC' and 'ASC'

Copy link
Member

Choose a reason for hiding this comment

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

And this can't be streamlined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was existing code for the most part, the only change to the sort was that the UI now sends the sort order vs the controller pulling the value from the database every time.

I didn't want to refactor sort as it was out of scope for this PR and would make the review more difficult.

I can address this in a follow-up if you want, or do you want it as part of this PR?

Copy link
Member

Choose a reason for hiding this comment

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

Do a follow-up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, removed the sort code changes.

* @param string $view returns messages in requested view ('singleton' or 'threaded')
*
* @return JSONResponse
*
Expand All @@ -134,7 +136,9 @@
public function index(int $mailboxId,
?int $cursor = null,
?string $filter = null,
?int $limit = null): JSONResponse {
?int $limit = null,
?string $sort = null,
?string $view = null): JSONResponse {
try {
$mailbox = $this->mailManager->getMailbox($this->currentUserId, $mailboxId);
$account = $this->accountService->find($this->currentUserId, $mailbox->getAccountId());
Expand All @@ -143,17 +147,25 @@
}

$this->logger->debug("loading messages of mailbox <$mailboxId>");

$order = $this->preferences->getPreference($this->currentUserId, 'sort-order', 'newest') === 'newest' ? 'DESC': 'ASC';
if ($sort === null) {
$sort = $this->preferences->getPreference($this->currentUserId, 'sort-order', 'newest') === 'newest' ? IMailSearch::ORDER_NEWEST_FIRST: IMailSearch::ORDER_OLDEST_FIRST;

Check warning on line 151 in lib/Controller/MessagesController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/MessagesController.php#L150-L151 

Added lines #L150 - L151 were not covered by tests
} else {
$sort = $sort === 'newest' ? IMailSearch::ORDER_NEWEST_FIRST: IMailSearch::ORDER_OLDEST_FIRST;

Check warning on line 153 in lib/Controller/MessagesController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/MessagesController.php#L153 

Added line #L153 was not covered by tests
}
if ($view !== null) {
$view = $view === 'singleton' ? IMailSearch::VIEW_SINGLETON : IMailSearch::VIEW_THREADED;

Check warning on line 156 in lib/Controller/MessagesController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/MessagesController.php#L155-L156 

Added lines #L155 - L156 were not covered by tests
}
$messages = $this->mailSearch->findMessages(
Copy link
Member

Choose a reason for hiding this comment

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

Inline the var and you have a smaller diff ✨

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, Hopefully this is what you ment

$account,
$mailbox,
$sort,
$filter === '' ? null : $filter,
$cursor,
$limit,
$view
);

Check warning on line 166 in lib/Controller/MessagesController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/MessagesController.php#L158-L166 

Added lines #L158 - L166 were not covered by tests
return new JSONResponse(
$this->mailSearch->findMessages(
$account,
$mailbox,
$order,
$filter === '' ? null : $filter,
$cursor,
$limit
)
$messages

Check warning on line 168 in lib/Controller/MessagesController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/MessagesController.php#L168 

Added line #L168 was not covered by tests
);
}

Expand Down
1 change: 1 addition & 0 deletions lib/Controller/PageController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ public function index(): TemplateResponse {
'app-version' => $this->config->getAppValue('mail', 'installed_version'),
'external-avatars' => $this->preferences->getPreference($this->currentUserId, 'external-avatars', 'true'),
'layout-mode' => $this->preferences->getPreference($this->currentUserId, 'layout-mode', 'vertical-split'),
'layout-message-view' => $this->preferences->getPreference($this->currentUserId, 'layout-message-view', $this->config->getAppValue('mail', 'layout_message_view', 'threaded')),
'reply-mode' => $this->preferences->getPreference($this->currentUserId, 'reply-mode', 'top'),
'collect-data' => $this->preferences->getPreference($this->currentUserId, 'collect-data', 'true'),
'search-priority-body' => $this->preferences->getPreference($this->currentUserId, 'search-priority-body', 'false'),
Expand Down
5 changes: 5 additions & 0 deletions lib/Controller/SettingsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,4 +124,9 @@
return new JSONResponse([]);
}

public function setLayoutMessageView(string $value): JSONResponse {
$this->config->setAppValue('mail', 'layout_message_view', $value);
return new JSONResponse([]);

Check warning on line 129 in lib/Controller/SettingsController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/SettingsController.php#L127-L129 

Added lines #L127 - L129 were not covered by tests
}

}
34 changes: 19 additions & 15 deletions lib/Db/MessageMapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -807,21 +807,23 @@ public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, string $sor
} else {
$select = $qb->select(['m.id', 'm.sent_at']);
}

$selfJoin = $select->expr()->andX(
$select->expr()->eq('m.mailbox_id', 'm2.mailbox_id', IQueryBuilder::PARAM_INT),
$select->expr()->eq('m.thread_root_id', 'm2.thread_root_id', IQueryBuilder::PARAM_INT),
$select->expr()->orX(
$select->expr()->lt('m.sent_at', 'm2.sent_at', IQueryBuilder::PARAM_INT),
$select->expr()->andX(
$select->expr()->eq('m.sent_at', 'm2.sent_at', IQueryBuilder::PARAM_INT),
$select->expr()->lt('m.message_id', 'm2.message_id', IQueryBuilder::PARAM_STR),

$select->from($this->getTableName(), 'm');

if ($query->getStacked()) {
$selfJoin = $select->expr()->andX(
$select->expr()->eq('m.mailbox_id', 'm2.mailbox_id', IQueryBuilder::PARAM_INT),
$select->expr()->eq('m.thread_root_id', 'm2.thread_root_id', IQueryBuilder::PARAM_INT),
$select->expr()->orX(
$select->expr()->lt('m.sent_at', 'm2.sent_at', IQueryBuilder::PARAM_INT),
$select->expr()->andX(
$select->expr()->eq('m.sent_at', 'm2.sent_at', IQueryBuilder::PARAM_INT),
$select->expr()->lt('m.message_id', 'm2.message_id', IQueryBuilder::PARAM_STR),
),
),
),
);

$select->from($this->getTableName(), 'm')
->leftJoin('m', $this->getTableName(), 'm2', $selfJoin);
);
$select->leftJoin('m', $this->getTableName(), 'm2', $selfJoin);
}

if (!empty($query->getFrom())) {
$select->innerJoin('m', 'mail_recipients', 'r0', 'm.id = r0.message_id');
Expand Down Expand Up @@ -1007,7 +1009,9 @@ public function findIdsByQuery(Mailbox $mailbox, SearchQuery $query, string $sor
);
}

$select->andWhere($qb->expr()->isNull('m2.id'));
if ($query->getStacked()) {
$select->andWhere($qb->expr()->isNull('m2.id'));
}

if ($sortOrder === 'ASC') {
$select->orderBy('m.sent_at', $sortOrder);
Expand Down
7 changes: 6 additions & 1 deletion lib/Service/Search/MailSearch.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
* @param string|null $filter
* @param int|null $cursor
* @param int|null $limit
* @param string|null $view
*
* @return Message[]
*
Expand All @@ -91,7 +92,8 @@
string $sortOrder,
?string $filter,
?int $cursor,
?int $limit): array {
?int $limit,
?string $view): array {
if ($mailbox->hasLocks($this->timeFactory->getTime())) {
throw MailboxLockedException::from($mailbox);
}
Expand All @@ -103,6 +105,9 @@
if ($cursor !== null) {
$query->setCursor($cursor);
}
if ($view !== null) {
$query->setStacked($view === self::VIEW_THREADED);

Check warning on line 109 in lib/Service/Search/MailSearch.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/Search/MailSearch.php#L109 

Added line #L109 was not covered by tests
}
// In flagged we don't want anything but flagged messages
if ($mailbox->isSpecialUse(Horde_Imap_Client::SPECIALUSE_FLAGGED)) {
$query->addFlag(Flag::is(Flag::FLAGGED));
Expand Down
10 changes: 10 additions & 0 deletions lib/Service/Search/SearchQuery.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
class SearchQuery {
/** @var int|null */
private $cursor;

private bool $stacked = true;

/** @var Flag[] */
private $flags = [];
Expand Down Expand Up @@ -72,6 +74,14 @@
$this->cursor = $cursor;
}

public function getStacked(): bool {
return $this->stacked;
}

public function setStacked(bool $stacked): void {
$this->stacked = $stacked;

Check warning on line 82 in lib/Service/Search/SearchQuery.php

View check run for this annotation

Codecov / codecov/patch

lib/Service/Search/SearchQuery.php#L81-L82 

Added lines #L81 - L82 were not covered by tests
}

public function getMatch(): string {
return $this->match;
}
Expand Down
6 changes: 6 additions & 0 deletions lib/Settings/AdminSettings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ public function getForm() {
$this->config->getAppValue('mail', 'allow_new_mail_accounts', 'yes') === 'yes'
);

$this->initialStateService->provideInitialState(
Application::APP_ID,
'layout_message_view',
$this->config->getAppValue('mail', 'layout_message_view', 'threaded')
);

$this->initialStateService->provideInitialState(
Application::APP_ID,
'llm_processing',
Expand Down
31 changes: 31 additions & 0 deletions src/components/AppSettingsMenu.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,24 @@
{{ t('mail', 'Horizontal split') }}
</NcCheckboxRadioSwitch>

<h6>{{ t('mail', 'Message View Mode') }}</h6>
<div class="sorting">
<NcCheckboxRadioSwitch type="radio"
name="message_view_mode_radio"
value="threaded"
:checked="layoutMessageView"
@update:checked="setLayoutMessageView('threaded')">
{{ t('mail', 'Show all messages in thread') }}
</NcCheckboxRadioSwitch>
<NcCheckboxRadioSwitch type="radio"
name="message_view_mode_radio"
value="singleton"
:checked="layoutMessageView"
@update:checked="setLayoutMessageView('singleton')">
{{ t('mail', 'Show only the selected message') }}
</NcCheckboxRadioSwitch>
</div>

<h6>{{ t('mail', 'Sorting') }}</h6>
<div class="sorting">
<NcCheckboxRadioSwitch class="sorting__switch"
Expand Down Expand Up @@ -398,6 +416,9 @@ export default {
layoutMode() {
return this.mainStore.getPreference('layout-mode', 'vertical-split')
},
layoutMessageView() {
return this.mainStore.getPreference('layout-message-view')
},
},
watch: {
showSettings(value) {
Expand Down Expand Up @@ -439,6 +460,16 @@ export default {
Logger.error('Could not save preferences', { error })
}
},
async setLayoutMessageView(value) {
try {
await this.mainStore.savePreference({
key: 'layout-message-view',
value,
})
} catch (error) {
Logger.error('Could not save preferences', { error })
}
},
async onOpen() {
this.showSettings = true
},
Expand Down
4 changes: 4 additions & 0 deletions src/components/Thread.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ export default {
return []
}

if (this.mainStore.getPreference('layout-message-view', 'threaded') === 'singleton') {
return [envelope]
}

const envelopes = this.mainStore.getEnvelopesByThreadRootId(envelope.accountId, envelope.threadRootId)
if (envelopes.length === 0) {
return []
Expand Down
35 changes: 35 additions & 0 deletions src/components/settings/AdminSettings.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,36 @@
</article>
<MicrosoftAdminOauthSettings :tenant-id="microsoftOauthTenantId" :client-id="microsoftOauthClientId" />
</div>
<div class="app-description">
<h3>{{ t('mail', 'User Interface Preference Defaults') }}</h3>
<article>
<p>
{{ t('mail', 'These settings are used to pre-configure the user interface preferences they can be overridden by the user in the mail settings') }}
</p>
</article>
<br>
<article>
<p>
{{ t('mail', 'Message View Mode') }}
</p>
<p>
<NcCheckboxRadioSwitch type="radio"
name="message_view_mode_radio"
value="threaded"
:checked.sync="layoutMessageView"
@update:checked="setLayoutMessageView('threaded')">
{{ t('mail', 'Show all messages in thread') }}
</NcCheckboxRadioSwitch>
<NcCheckboxRadioSwitch type="radio"
name="message_view_mode_radio"
value="singleton"
:checked.sync="layoutMessageView"
@update:checked="setLayoutMessageView('singleton')">
{{ t('mail', 'Show only the selected message') }}
</NcCheckboxRadioSwitch>
</p>
</article>
</div>
</SettingsSection>
</template>

Expand All @@ -278,6 +308,7 @@ import {
updateLlmEnabled,
updateEnabledSmartReply,
setImportanceClassificationEnabledByDefault,
setLayoutMessageView,
} from '../../service/SettingsService.js'

const googleOauthClientId = loadState('mail', 'google_oauth_client_id', null) ?? undefined
Expand Down Expand Up @@ -342,6 +373,7 @@ export default {
isLlmFreePromptConfigured: loadState('mail', 'enabled_llm_free_prompt_backend'),
isClassificationEnabledByDefault: loadState('mail', 'llm_processing', true),
isImportanceClassificationEnabledByDefault: loadState('mail', 'importance_classification_default', true),
layoutMessageView: loadState('mail', 'layout_message_view'),
}
},
methods: {
Expand Down Expand Up @@ -410,6 +442,9 @@ export default {
logger.error('Could not save default classification setting', { error })
}
},
async setLayoutMessageView(value) {
await setLayoutMessageView(value)
},
},
}
</script>
Expand Down
4 changes: 4 additions & 0 deletions src/init.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ export default function initAfterAppCreation() {
key: 'layout-mode',
value: preferences['layout-mode'],
})
mainStore.savePreferenceMutation({
key: 'layout-message-view',
value: preferences['layout-message-view'],
})
mainStore.savePreferenceMutation({
key: 'follow-up-reminders',
value: preferences['follow-up-reminders'],
Expand Down
8 changes: 7 additions & 1 deletion src/service/MessageService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export function fetchEnvelope(accountId, id) {
})
}

export function fetchEnvelopes(accountId, mailboxId, query, cursor, limit) {
export function fetchEnvelopes(accountId, mailboxId, query, cursor, limit, sort, view) {
const url = generateUrl('/apps/mail/api/messages')
const params = {
mailboxId,
Expand All @@ -47,6 +47,12 @@ export function fetchEnvelopes(accountId, mailboxId, query, cursor, limit) {
if (cursor) {
params.cursor = cursor
}
if (sort) {
params.sort = sort
}
if (view) {
params.view = view
}

return axios
.get(url, {
Expand Down
Loading
Loading