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

LEAN-4054: update the export endpoint to use the live pm doc #163

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

Conversation

mbetamony
Copy link
Contributor

No description provided.

@mbetamony mbetamony requested a review from mnatsheh October 14, 2024 19:13
@mnatsheh
Copy link
Contributor

We still need to support the exporting with the most recent snapshot. Also, do we need to worry about applying the changes before using the doc?

@mbetamony
Copy link
Contributor Author

We still need to support the exporting with the most recent snapshot.

So we're going to accept a param to use the snapshot or the current doc?

Also, do we need to worry about applying the changes before using the doc?

As far as I know we technically don't, the data used for the export is the updated content. (I'll need to confirm this, I'm not entirely sure)

@mnatsheh
Copy link
Contributor

So we're going to accept a param to use the snapshot or the current doc?

Probably. It should be an optional param with the current behavior (using the latest snapshot) as the default.

As far as I know we technically don't, the data used for the export is the updated content. (I'll need to confirm this, I'm not entirely sure)

What's actually unclear is whether we need pending suggestions to show up. Can you check what the behavior will be in that case (try deleting some text without approving and see the output jats)? I'll update the ticket with this question.

@mbetamony
Copy link
Contributor Author

So we're going to accept a param to use the snapshot or the current doc?

Probably. It should be an optional param with the current behavior (using the latest snapshot) as the default.

As far as I know we technically don't, the data used for the export is the updated content. (I'll need to confirm this, I'm not entirely sure)

What's actually unclear is whether we need pending suggestions to show up. Can you check what the behavior will be in that case (try deleting some text without approving and see the output jats)? I'll update the ticket with this question.

Yeah the output jats is weird, here's an example:

<p id="p-1">
    <del>Patients with schizophrenia are at a higher risk for suicide compared with the general population. Dopamine-beta-hydroxylase (D?H) plays a key role in the conversion of dopamine to norepinephrine, which is related to suicidal behavior and cognitive regulation.</del>
</p>
<p id="p-2">
    <del>To examine whethe</del>
    r there is the effect of <italic>D?H</italic>
    5? <italic>-</italic>
    insertion/deletion (Ins/Del) polymorphism on cognitive performance in suicide attempters with chronic schizophrenia.
</p>

@mbetamony
Copy link
Contributor Author

mbetamony commented Oct 20, 2024

Patients with schizophrenia are at a higher risk for suicide compared with the general population. Dopamine-beta-hydroxylase (D?H) plays a key role in the conversion of dopamine to norepinephrine, which is related to suicidal behavior and cognitive regulation.

For some reason i thought the text insertion/deletion (Ins/Del) was there because of the track changes, but that was of part of the xml 👀.

But to answer your question, deleting text that isn't approved will show under del tags; check the mark rules we have in the export and similarly the inserted text will show under ins tags.

It looks like based on Nick's comment that we don't want to have pending suggestions; meaning we don't want tracked_insert text and we want to show the tracked_delete text, correct?
Edit: Probably its not that straight forward, we have to think about other type of content (e.g. inserting a table)

@mnatsheh
Copy link
Contributor

What about blocks? Does a deleted block also show under a del tag? I think this will get quite complicated, so we should just ignore for now, and just use the doc as is.

@mbetamony
Copy link
Contributor Author

What about blocks? Does a deleted block also show under a del tag? I think this will get quite complicated

Its handled as regular content (its not actually deleted and its not under a del tag). No marks attrs are added.

so we should just ignore for now, and just use the doc as is.

Got it

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.

2 participants