Feat/duplication #1180

mrflos · 2024-07-01T10:29:56Z

Description of pull request / Description de la demande d'ajout

90% finished feature of duplication of pages/lists in same wiki or to external wikis

After rebase from doryphore-dev i had some strange bugs, i tried to fix them, hopefully i got them all.

You can test the duplication feature on another wiki by giving url local dev url as url for duplication (it will use the api roads instead of the controller).

Sorry for the quite big codebase changes...

Edit: i can see what when wrong : there are two other beta-features included : an image optimizer, and extra-fields feature (that add comments and reactions to bazarlist if needed)
I will check if everything is working as expected, as it should also be reviewed, or do you prefer separated PRs ?

javascripts/handlers/duplicate.js

…uplication

seballot

WIP review, j'ai l'impression que github a perdu la moitié de mes commentaires..

-> edit c'est bon il les a gardé, je continue alors

seballot · 2024-07-01T14:28:52Z

templates/handlers/duplicate.twig

+    <button name="duplicate-action" value="open" type="submit" class="btn btn-primary"><i class="fa fa-eye-open"></i>{{ _t('DUPLICATE_AND_OPEN') }}</button>
+    <button name="duplicate-action" value="edit" type="submit" class="btn btn-primary"><i class="fa fa-pencil-alt"></i> {{ _t('DUPLICATE_AND_EDIT') }}</button>
+    <button name="duplicate-action" value="return" type="submit" class="btn btn-primary"><i class="fa fa-arrow-left"></i> {{ _t('DUPLICATE_AND_RETURN') }}</button>


Je trouve que ça complique l'interface d'avoir ces trois boutons, sans apporter de grand bénéfice à l'utilisateur

Je pense qu'on devrait toujours faire "duplicate & afficher" (pas besoin d'icone), et si ils veulent éditer ben ils cliquent éditer, si ils veulent revenir en arrière ils cliquent le bouton dédié dans le navigateur

ok je demande l'avis de @furax37 au cas ou, mais pas d'objection

seballot · 2024-07-01T14:29:48Z

handlers/DuplicateHandler.php

+            ]);
+        } elseif (!$this->getService(AclService::class)->hasAccess('read', $this->wiki->GetPageTag())) {
+            // if no read access to the page
+            if ($contenu = $this->getService(PageManager::class)->getOne("PageLogin")) {


le gars fait la description de sa PR en anglais mais utilise du francais dans le code, gnahaha :P

un peu de french touch dans la codebase!

handlers/DuplicateHandler.php

seballot · 2024-07-01T14:35:14Z

includes/controllers/ApiController.php

+        $duplicationManager = $this->getService(DuplicationManager::class);
+        try {
+            $duplicationManager->importDistantContent($tag, $request);
+        } catch (\Throwable $th) {


pour info comme y'a un use Throwable y'a pas besoin du \

seballot · 2024-07-01T14:35:45Z

includes/controllers/ApiController.php

@@ -16,14 +16,13 @@
 use YesWiki\Core\Service\AclService;
 use YesWiki\Core\Service\ArchiveService;
 use YesWiki\Core\Service\CommentService;
-use YesWiki\Core\Service\DbService;


c'est une erreur d'avoir retiré cet import, c'est utilisé. J'ai fait un fix

je croies que j'avais corrigé ca et le missing { mais j'ai pas du bien reprendre mon commit lors du cherry-pick

seballot · 2024-07-01T15:07:13Z

includes/services/DuplicationManager.php

+            // find fields that are textareas
+            foreach ($form['prepared'] as $field) {
+                if ($field instanceof TextareaField or $field instanceof ImageField or $field instanceof FileField) {
+                    $fields[] = [


je pense qu'il vaudrait mieux retourner le $field entrier, plutot que ce tableau avec juste ID et TYPE, comme ça on sait ce que contient la variable juste avec le nom (un $field c'est un BazarField, pas un array [id, type]) et aussi plus loin dans le code ça évite des check un foireux du genre $field[type] == 'image', c'est mieux de refaire un $field instance of ImageField

seballot · 2024-07-01T15:08:18Z

includes/services/ImportFilesManager.php

heu cette classe est pas utilisée non? on dirait que les function sont déjà dans DuplicateManager

seballot · 2024-07-01T15:09:41Z

includes/services/PageManager.php

@@ -298,6 +297,7 @@ public function deleteOrphaned($tag)
        $this->dbService->query("DELETE FROM {$this->dbService->prefixTable('links')} WHERE from_tag='{$this->dbService->escape($tag)}' ");
        $this->dbService->query("DELETE FROM {$this->dbService->prefixTable('acls')} WHERE page_tag='{$this->dbService->escape($tag)}' ");
        $this->dbService->query("DELETE FROM {$this->dbService->prefixTable('triples')} WHERE `resource`='{$this->dbService->escape($tag)}' and `property`='" . TripleStore::TYPE_URI . "' and `value`='" . EntryManager::TRIPLES_ENTRY_ID . "'");
+        $this->dbService->query("DELETE FROM {$this->dbService->prefixTable('triples')} WHERE `resource`='{$this->dbService->escape($tag)}' and `property`='" . TripleStore::TYPE_URI . "' and `value`='" . EntryManager::TRIPLES_ENTRY_ID . "'");


pourquoi avoir dupliqué cette ligne?? si y'a une raison spécifique mieux vaut ajouter un commentaire pour expliquer parce ça fait bizarre :)

je pense que c'est mon rebase foireux, il a du prendre les deux

seballot · 2024-07-01T15:10:32Z

includes/services/PageManager.php

        }, $pages);

        return $pages;
    }
+
+    private function duplicate($sourceTag, $destinationTag): bool


?? un code de test? à supprimer non?

non je voulais creer une methode dans le PageManager, mais work in progress

seballot · 2024-07-01T15:11:20Z

javascripts/handlers/duplicate.js

+function handleLoginResponse(data) {
+  if (data.isAdmin === true) {
+    $('#login-message').html(_t('CONNECTED_AS_ADMIN', { user: data.user })).parents('.form-group')
+      .removeClass('has-error')


ah ah, old school ! on a VueJs maintenant hein :P

je veux bien tenter une conversion en vue, mais je sens qu'il va falloir charger une lib pour les requetes ajax, mettre des methodes a la con etc, mais bon pourquoi pas.

non non laisse ton code comme ça, maintenant qu'il est écrit ne va pas passer du temps à déjà le refactor !

seballot

Yo !

top tout ça !

J'ai fait des petites remarques pour la forme, mais c'est pas du tout bloquant. y'a 2/3 commentaires à fixer cependant

J'ai poussé les petit fix, plus un passage de linter (ils étaient pas en place quand tu as commencé le boulot sur cette branche)

seballot · 2024-07-01T15:19:22Z

templates/handlers/duplicate.twig

+    <div class="form-group">
+      <label for="urlWiki" class="control-label">{{ _t('WIKI_URL') }}</label>
+      <div class="input-group">
+        <input required type="url" class="form-control" id="urlWiki" name="urlWiki" placeholder="{{ _t('WIKI_URL') }}" value="" />


normalement on utilise du dash case pour les id aussi url-wiki

seballot · 2024-07-01T15:20:14Z

tools/bazar/services/EntryManager.php

@@ -1180,4 +1180,11 @@ function ($attributeName) {

        return $entriesIds;
    }
+
+    private function duplicate($sourceTag, $destinationTag): bool


pareil, test code?

pareil methode de service non existante encore..

seballot · 2024-07-01T15:21:21Z

tools/bazar/actions/BazarListeAction.php

@@ -417,4 +417,3 @@ private function getExternalUrlsFromIds(?string $ids)

        return $externalIds;
    }
-}


petite boulette manque le } maintenant ! j'ai fixé :)

j'avais corrigé ailleurs ca.. mais merci pour le fix

seballot · 2024-07-01T15:33:14Z

tools/bazar/templates/lists/list_table.twig

@@ -36,6 +36,11 @@
        </td>
        <td>{{ key }}</td>
        <td>
+          {% if list.canDuplicate %}
+          <a class="btn btn-default btn-xs" href="{{ url({ params: { vue: 'listes', action: 'duplicate', idliste: key }}) }}">
+            <i class="far fa-clone"></i>


On est d'accord c'est pas encore implémenté la duplication de liste? en tout cas chez moi ça marche pas :)

oui ca vient

javascripts/handlers/duplicate.js

+      if (btnAction == 'open') {
+        location = `${shortUrl}/?${data.pageTag}`
+      } else if (btnAction == 'edit') {
+        location = `${shortUrl}/?${data.pageTag}/edit`


mrflos · 2024-07-01T16:02:50Z

Ok merci pour cette revue, je vais tout prendre en compte, sauf peut etre la re--ecriture jquery vers vuejs, je ferai que si j'ai du temps.

mrflos requested a review from seballot July 1, 2024 10:29

mrflos marked this pull request as draft July 1, 2024 10:30

github-advanced-security bot found potential problems Jul 1, 2024

View reviewed changes

javascripts/handlers/duplicate.js Dismissed Show dismissed Hide dismissed

javascripts/handlers/duplicate.js Fixed Show fixed Hide fixed

mrflos added 14 commits July 1, 2024 12:04

wip duplication

caa3a30

feat(duplication) : wip duplication handler

772c1a8

feat(duplication): local duplication is almost finished

649a1f7

feat(duplicate): works for entries and rename manager

2f81288

feat(duplication): duplicate acls and metadatas

5596396

feat(duplication): make entry duplicate and change field content

041630f

feat(duplication): wip UI for external duplication

13aa5c6

feat(duplication): only admins can duplicate

4b18051

style: make toast global to avoid linting errors

a5b0869

feat(duplication): auth routes and verify url and login for distant d…

2630656

…uplication

feat(duplication): wip external duplication UI

fb02c12

wip distant duplication new api routes and ux

8bb6841

feat(duplication) : wip duplication handler

5b4abc7

feat(duplication): external page duplication

301df7f

seballot force-pushed the feat/duplication branch from a9c4a8b to 301df7f Compare July 1, 2024 12:13

mrflos added 2 commits July 1, 2024 15:39

fix(controllers): post rebase strange bugs

94cb65d

fix(duplication): netry duplication on distant wiki was not working

c1b23a1

seballot reviewed Jul 1, 2024

View reviewed changes

seballot added 3 commits July 1, 2024 15:26

Fix missing import

c599e51

fix missing }

84019bd

lint

eba365d

seballot approved these changes Jul 1, 2024

View reviewed changes

github-advanced-security bot found potential problems Jul 1, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/duplication #1180

Feat/duplication #1180

mrflos commented Jul 1, 2024 •

edited

Loading

seballot left a comment •

edited

Loading

seballot Jul 1, 2024

mrflos Jul 1, 2024

seballot Jul 1, 2024

mrflos Jul 1, 2024

seballot Jul 1, 2024

seballot Jul 1, 2024

mrflos Jul 1, 2024

seballot Jul 1, 2024

seballot Jul 1, 2024

seballot Jul 1, 2024

mrflos Jul 1, 2024

seballot Jul 1, 2024

mrflos Jul 1, 2024

seballot Jul 1, 2024

mrflos Jul 1, 2024

seballot Jul 1, 2024

seballot left a comment

seballot Jul 1, 2024

seballot Jul 1, 2024

mrflos Jul 1, 2024

seballot Jul 1, 2024

mrflos Jul 1, 2024

seballot Jul 1, 2024

mrflos Jul 1, 2024

mrflos commented Jul 1, 2024 •

edited

Loading

Feat/duplication #1180

Are you sure you want to change the base?

Feat/duplication #1180

Conversation

mrflos commented Jul 1, 2024 • edited Loading

Description of pull request / Description de la demande d'ajout

seballot left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seballot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrflos commented Jul 1, 2024 • edited Loading

mrflos commented Jul 1, 2024 •

edited

Loading

seballot left a comment •

edited

Loading

mrflos commented Jul 1, 2024 •

edited

Loading