-
Notifications
You must be signed in to change notification settings - Fork 17
Add file actions #312
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
Add file actions #312
Conversation
6fc2627
to
46b1fb4
Compare
@marcoambrosini @jancborchardt I made a first implementation. Let's adjust it a bit. Once it is clicked, there's a top-right toast saying "A file will be created soon". Questions:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome awesome @julien-nc! :)
Questions:
* Are you fine with the action names and icons?
The names are good! Best to use the Material Symbols for it:
- Summarize: https://fonts.google.com/icons?selected=Material+Symbols+Outlined:summarize:FILL@0;wght@400;GRAD@0;opsz@24&icon.query=summariz&icon.size=24&icon.color=%23e3e3e3
- Text to speech: https://fonts.google.com/icons?selected=Material+Symbols+Outlined:speech_to_text:FILL@0;wght@400;GRAD@0;opsz@24&icon.query=transcri&icon.size=24&icon.color=%23e3e3e3
- Transcribe audio: Better icon → https://fonts.google.com/icons?selected=Material+Symbols+Outlined:speech_to_text:FILL@0;wght@400;GRAD@0;opsz@24&icon.query=transcri&icon.size=24&icon.color=%23e3e3e3
* Should we adjust the toast msg?
Can you check for the wording @marcoambrosini? It's about the showSuccess and showError texts I assume.
* Are you fine with the behaviour or creating a file next to the source one? * Are you fine with the suffix?
Sure, seems great to put it next to it. Just the naming should be adjusted to be human readable:
- " - summarized"
- " - text to speech"
- " - transcribed"
* Should there be a notification when the new file is created?
Good question, I guess you should be notified when it's done just like the Talk recording.
And one more thing: What about direct transcription + summarization? Would be a nice feature too and could just chain the 2 functions in the back?
829e56d
to
6f5560d
Compare
@jancborchardt Thanks for the feedback.
![]()
I don't have time for that these days. We can keep it as a followup task. (Maybe it's worth creating another GH issue for that). |
6f5560d
to
7b4d1ae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Tested it out and it worked.
Just a nitpick and some things that should be discussed.
return t('assistant', 'Assistant') | ||
}, | ||
enabled(nodes, view) { | ||
return !actionIgnoreLists.includes(view.id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be disabled if none of the assistant tasks are available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the case, the group action is not registered if none of the task types are available. Check the end of src/files/fileActions.js
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had meant for filetypes that currently don't have an assistant file action.
&& nodes.length === 1 | ||
&& !nodes.some(({ permissions }) => (permissions & Permission.READ) === 0) | ||
&& nodes.every(({ type }) => type === FileType.File) | ||
&& nodes.every(({ mime }) => ['text/plain', 'text/markdown'].includes(mime)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing here as for summarize
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, instead of toggling the action with its enabled
attr, we conditionally register it at the end of the file.
&& nodes.length === 1 | ||
&& !nodes.some(({ permissions }) => (permissions & Permission.READ) === 0) | ||
&& nodes.every(({ type }) => type === FileType.File) | ||
&& nodes.every(({ mime }) => ['text/plain', 'text/markdown'].includes(mime)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth it to support all files that parseTextFromFile supports instead of just markdown and plain text.
assistant/lib/Service/AssistantService.php
Lines 655 to 725 in 0051c8e
public function parseTextFromFile(string $userId, ?string $filePath = null, ?int $fileId = null): string { | |
try { | |
$userFolder = $this->rootFolder->getUserFolder($userId); | |
} catch (\OC\User\NoUserException|NotPermittedException $e) { | |
throw new \Exception('Could not access user storage.'); | |
} | |
try { | |
if ($filePath !== null) { | |
$file = $userFolder->get($filePath); | |
} else { | |
$file = $userFolder->getFirstNodeById($fileId); | |
} | |
} catch (NotFoundException $e) { | |
throw new \Exception('File not found.'); | |
} | |
try { | |
if ($file instanceof File) { | |
$fileContent = $file->getContent(); | |
} else { | |
throw new \Exception('Provided path does not point to a file.'); | |
} | |
} catch (LockedException|GenericFileException|NotPermittedException $e) { | |
throw new \Exception('File could not be accessed.'); | |
} | |
$mimeType = $file->getMimeType(); | |
switch ($mimeType) { | |
default: | |
case 'text/plain': | |
{ | |
$text = $fileContent; | |
break; | |
} | |
case 'text/markdown': | |
{ | |
$parser = new Parsedown(); | |
$text = $parser->text($fileContent); | |
// Remove HTML tags: | |
$text = strip_tags($text); | |
break; | |
} | |
case 'text/rtf': | |
{ | |
$text = $this->parseRtfDocument($fileContent); | |
break; | |
} | |
case 'application/vnd.openxmlformats-officedocument.wordprocessingml.document': | |
case 'application/msword': | |
case 'application/vnd.oasis.opendocument.text': | |
{ | |
$tempFilePath = $this->tempManager->getTemporaryFile(); | |
file_put_contents($tempFilePath, $fileContent); | |
$text = $this->parseDocument($tempFilePath, $mimeType); | |
$this->tempManager->clean(); | |
break; | |
} | |
case 'application/pdf': | |
{ | |
$parser = new Parser(); | |
$pdf = $parser->parseContent($fileContent); | |
$text = $pdf->getText(); | |
break; | |
} | |
} | |
return $text; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense!
I created an issue to keep track of that: #321
We can do it later, after having merged this one.
7b4d1ae
to
e50a3f1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, let’s do the rest in follow-ups! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the clarification on what I meant with disabling the group action when there is no task available for that file everything else LGTM
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
…e material symbols Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
…d action button in notifications to open the result task in the assistant Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
6277a0b
to
9d99e95
Compare
@kyteinsky Thanks for the review. All done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
closes #304
3 File actions are registered:
A notification is sent after a file action process finishes or fails.
Main changes: