Skip to content
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

Get notified on changes related to a corpus #74

Open
wants to merge 3 commits into
base: v4
Choose a base branch
from

Conversation

sarah-ngn
Copy link

@sarah-ngn sarah-ngn commented Jun 21, 2021

FEATURE : Get notified on corpus changes.
Co-authored-by: Thomas Godard thomas.godard@utt.fr
Check Hypertopic/Porphyry#501 for more details
@benel

Copy link
Member

@benel benel left a comment

Choose a reason for hiding this comment

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

Merci pour cette contribution @sarah-ngn et @lildelfino.
On n'est pas loin d'avoir une fonctionnalité !

J'ai noté dans les commentaires ligne à ligne quelques améliorations à apporter.

Quand tout ceci sera fait, n'oubliez pas ensuite de présenter tout ça sous la forme d'un unique commit :

  • dans la première ligne :
    • avec préfixe standard (FEATURE: ),
    • titre officiel en anglais (Get notified on corpus changes.),
  • dans la troisième ligne, mention du co-auteur sous la forme Co-authored-by: Prenom Nom <e-mail@example.com>,
  • dans la quatrième ligne, référence au ticket de Porphyry (ex : Check Hypertopic/Porphyry#501 for more details).

app/lists/feed.js Outdated Show resolved Hide resolved
app/lists/feed.js Outdated Show resolved Hide resolved
app/views/modified/map.js Outdated Show resolved Hide resolved
app/lists/feed.js Outdated Show resolved Hide resolved
app/lists/feed.js Outdated Show resolved Hide resolved
app/lists/feed.js Outdated Show resolved Hide resolved
app/lists/feed.js Outdated Show resolved Hide resolved
@sarah-ngn
Copy link
Author

@benel
Nous avons effectués les modifications dans un deuxième commit.

@lildelfino
Copy link

@benel
Nous avons commit en prenant compte du travail de @cedricleandres et en ajoutant un identifiant unique dans la balise

Copy link
Member

@benel benel left a comment

Choose a reason for hiding this comment

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

Merci @sarah-ngn @lildelfino !
On y est presque !

Vous trouverez dans les commentaires ligne à ligne quelques dernières demandes de modification.

Une fois que c'est fait, merci de tout remettre dans un seul commit sous la forme indiquée précédemment dans la discussion.

Ça va aller ?

'<title>' + row.doc.item_name + '</title>',
'<description> Lieu : ' + row.doc.spatial + ' Créé le : ' + row.doc.created + ' Catégorie(s) : ' + topic + '</description>',
'<link>' + uri + '/item/' + row.doc.item_corpus + '/' + row.id + '</link>',
'<guid>Modifié le : ' + row.doc.record.modified + '</guid>'
Copy link
Member

Choose a reason for hiding this comment

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

Même si ça devrait marcher, c'est un peu bizarre de mettre une phrase (en français qui plus est ) comme identifiant.

Par contre, vous avez raison @sarah-ngn, le moment précis de la modification est un bon candidat pour identifier l'évènement.

'Content-Type': 'text/xml'
}
});
uri = req.query.app;
Copy link
Member

Choose a reason for hiding this comment

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

Attention la variable n'est pas déclarée. Même si ça fonctionne, ça peut causer des effets de bord.

send(uri);
send('</link><description>Created or updated items.</description>');
while(row = getRow()){
var i=0;
Copy link
Member

Choose a reason for hiding this comment

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

Il est souvent conseillé de mettre un espace de chaque côté du =.

send('</link><description>Created or updated items.</description>');
while(row = getRow()){
var i=0;
var topic="";
Copy link
Member

Choose a reason for hiding this comment

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

Il est souvent conseillé de mettre un espace de chaque côté du =.

var i=0;
var topic="";
for (var t in row.doc.topics){
topic+=t+", ";
Copy link
Member

Choose a reason for hiding this comment

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

Il est souvent conseillé de mettre un espace de chaque côté du += et du +.

"from": ":object/:resource",
"to": "../../:object/:resource"
}
]
Copy link
Member

Choose a reason for hiding this comment

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

Attention, vous avez dû modifier quelque chose dans l'indendation (peut-être est-ce à cause de l'éditeur que vous utilisez). Du coup, ça rend difficile de voir précisément ce que vous vouliez réellement modifier.

En plus des problèmes de lecture du diff ça rend toutes les autres opérations de Git moins efficaces (merge, revert, blame, bisect...).

function (doc) {
if (doc.item_corpus && doc.record.modified) {
emit([doc.item_corpus, doc.record.modified]);
if(doc.topics && doc.record.modified){
Copy link
Member

@benel benel Jun 25, 2021

Choose a reason for hiding this comment

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

Il est conseillé de mettre un espace après le if ainsi que avant l'accolade.

@benel benel changed the title IMPLEMENTATION : vue, list et rewrite pour les items modifiés du corpus Get notified on corpus changes Apr 22, 2022
@benel benel changed the title Get notified on corpus changes Get notified on changes related to a corpus Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants