Skip to content

SCENARIO: Linking an existing document (see #383).#392

Open
Come-Peyrelongue wants to merge 2 commits into
mainfrom
fix-383
Open

SCENARIO: Linking an existing document (see #383).#392
Come-Peyrelongue wants to merge 2 commits into
mainfrom
fix-383

Conversation

@Come-Peyrelongue
Copy link
Copy Markdown

@Come-Peyrelongue Come-Peyrelongue commented Mar 24, 2026

Updated the scenario to include comments on video documents and modified the expected document list.

Co-authored-by: Enzo MOUGIN enzo.mougin@utt.fr
Co-authored-by: Côme PEYRELONGUE come.peyrelongue@utt.fr
Co-authored-by: Sacha HIMBER sacha.himber@utt.fr

We, Enzo MOUGIN, Côme PEYRELONGUE & Sacha HIMBER, hereby grant to Hyperglosae maintainers the right to publish our contribution under the terms of any licenses the Free Software Foundation classifies as Free Software Licenses.

@benel
Copy link
Copy Markdown
Member

benel commented Mar 31, 2026

@Come-Peyrelongue @LeMouj @sachaS456
Thank you for your contribution.

It seems that your Co-authored-by was not recognized by GitHub.
Please check the exact syntax in the documentation.

I strongly recommend using Git Fork for the intensive git history rewriting done in IF05.

@benel
Copy link
Copy Markdown
Member

benel commented Mar 31, 2026

Please note also that the pull request should be in draft mode until it is complete (with scenario, tests and implementation).

@Come-Peyrelongue Come-Peyrelongue marked this pull request as ready for review April 7, 2026 13:30
@Come-Peyrelongue Come-Peyrelongue requested a review from benel as a code owner April 7, 2026 13:30
@benel
Copy link
Copy Markdown
Member

benel commented Apr 8, 2026

Thank you for your contribution @sachaS456 @Come-Peyrelongue @LeMouj

Please don't forget that your pull request won't be integrated if your commits they don't follow strict naming conventions.

It is quite easy with Git-Fork. See how it was done for a different issue: #390 (comment)
If you have any questions about these vidéos, just ask me.

Please note that the videos also show how Co-authored-by GitHub syntax should be used in commits.

Copy link
Copy Markdown
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.

@Come-Peyrelongue @sachaS456 @LeMouj

Thank you very much for your contribution. Please consider my review so that acceptance tests fully comply with BDD.

Regards.

Comment thread frontend/tests/outcome.js Outdated
Comment on lines +211 to +214
cy.get('.document-list-container .existingDocument')
.should('have.length.greaterThan', 0)
.each(($card) => {
cy.wrap($card).find('span').invoke('text').should('match', /\S+/);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As explained in the comment of the scenario, move this condition into Quand je réutilise {string} comme glose.

"Bolo to raz" (svk)
"Comments on the video - Côme Peyrelongue, 24/03/2026 09:32 - My First Video Document"
"My Second Video Document - Côme Peyrelongue, 10/03/2026 20:15"
"Comments on the video - Côme Peyrelongue, 23/03/2026 10:44 - My Second Video Document"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • I see no reason to remove the existing scenario. It is really useful to check that its contents is linked and displayed.
  • I don't understand how you can be sure that everyone with just the code base, at any step of the tests will have your own documents. Nothing in the scenario tells that you should have them. And nothing in tests or data samples can make this assumption true.
  • Because your improvement is only a small part of Quand je réutilise {string} comme glose, you don't have to change the scenario: you just have to add conditions into the implementation of your test step.

It is important to understand all three objections. However if you take into account the third one it will fix the other two.

@benel
Copy link
Copy Markdown
Member

benel commented May 13, 2026

Thank you for your updates @sachaS456 @Come-Peyrelongue @LeMouj

Please note that one of your acceptance tests failed:

  1. Relier un document existant
    pour commenter un document:
    AssertionError: Timed out retrying after 6000ms: Expected to find element: .existingDocument, but never found it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Good first issue Issue that is easy to test and fix for new team members or external contributors

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants