accordproject / cicero-word-add-in

A plugin to allow users to interact with Accord Project templates directly in MS Word.
Apache License 2.0
19 stars 9 forks source link

Persistent Custom XML #45

Open K-Kumar-01 opened 3 years ago

K-Kumar-01 commented 3 years ago

Bug Report 🐛

Currently, the custom XML namespace when a template is inserted is persistent i.e. it does not change when a template is removed from the document.

Expected Behavior

When we remove a template from the document, the XML of the templateIdentifier stored in custom XML should also get removed. The XML looks similar to this.

<?xml version="1.0" encoding="utf-8" ?>
<templates xmlns="https://accordproject.org/">
  <template xmlns="acceptance-of-delivery@0.14.0" />
  ...
</templates>

If one deletes the template acceptance-of-delivery then this identifier should also get removed.

Current Behavior

The identifier remains there and is not deleted.

Possible Solution

We should remove the identifiers whenever a template is deleted.

Steps to Reproduce

After the add-in server is running and the npm start command is entered.

  1. Insert any 2 different templates.
  2. Save the document as WORD XML format.
  3. Now open the XML in any editor(Vs-Code, atom) and see the identifier of 2 different templates. (find for <template xmlns in the file)
  4. 2 different occurrences will be found
  5. Now in the document delete any of the templates and save the document as XML format.
  6. Repeat step 3.
  7. Still two different occurrences are found.

Context (Environment)

Desktop

Detailed Description

The change will ensure that only those identifiers are present whose templates are present in the document.

Possible Implementation

According to this, there is a content-control onDeleted which we can use to delete the identifier of the template.

Would you like to work on the issue?

Yes

algomaster99 commented 3 years ago

This is a very good issue! Looking forward to PR for this.

K-Kumar-01 commented 3 years ago

Going to work on this now

K-Kumar-01 commented 3 years ago

@algomaster99 I need some help with this issue. I was trying to use this one. However, it states that API should not be used in a production environment. So shall we go with this?

Also, I had some difficulties setting this up in the code.

algomaster99 commented 3 years ago

I would try the following approaches:

  1. Use delete with keepContent set to false. This API can be triggered from the Word Add-in (maybe the document panel). We can instruct a user to not delete the clause by deleting the content control directly but instead we can render a button in document panel which will trigger the above mentioned API.
  2. Similar to the above approach, you can first use the delete API for removing the CustomXMLPart and then you can remove the content control. Again, the user is expected to click a button and launch this API.
  3. I am not sure about this but maybe we can again use Office.EventType.BindingDataChanged. Whenever a user deletes the content of the content control, this event will launch and delete the XML too. However, this might be unsafe as user can delete the content control directly so this event would never trigger in that case. Moreover, I remember that I have made the content inside a content control not editable.

I agree the best approach would have been what you mentioned but as far as I recollect these preview APIs don't work even in the development environment. Anyway, they have suggested to not use it for now so let's keep it that way.

K-Kumar-01 commented 3 years ago

I would try the following approaches:

  1. Use delete with keepContent set to false. This API can be triggered from the Word Add-in (maybe the document panel). We can instruct a user to not delete the clause by deleting the content control directly but instead we can render a button in document panel which will trigger the above mentioned API.
  2. Similar to the above approach, you can first use the delete API for removing the CustomXMLPart and then you can remove the content control. Again, the user is expected to click a button and launch this API.
  3. I am not sure about this but maybe we can again use Office.EventType.BindingDataChanged. Whenever a user deletes the content of the content control, this event will launch and delete the XML too. However, this might be unsafe as user can delete the content control directly so this event would never trigger in that case. Moreover, I remember that I have made the content inside a content control not editable.

I agree the best approach would have been what you mentioned but as far as I recollect these preview APIs don't work even in the development environment. Anyway, they have suggested to not use it for now so let's keep it that way.

@algomaster99 Thanks for the help and clarification. I will try to implement and see if we are able to find an effective solution from these.

K-Kumar-01 commented 3 years ago

@algomaster99 Based on what you suggested, I have thought of a particular solution. Let me know if this can be done.

  1. In the Document component, we will keep track of the inserted templates.
  2. Also a state templatesIns can be made to keep track of OOXML of inserted templates. So the complete OOXML of the templates can be rendered using this.
  3. The templates can be kept non-deletable in the beginning. When we click on the document component, we can delete the templates from there using OfficeJs API.
  4. Also using the 3, we can delete the XML part present.

In this way, we can ensure users delete from one place only and insert from one place only thereby providing some safety.

The deletable thing can also be switched with add to document instead of the document.

if(template)render delete_template;
else render add_to_template

MAJOR DRAWBACK: If a user types anything other than the template and then deletes any template, the typed thing will vanish.

Since the debugger is not working, I am not able to debug my approach. As a result, I have posted it here to get some feedback on whether we should proceed with this or not?

algomaster99 commented 3 years ago

Also a state templatesIns can be made to keep track of OOXML of inserted templates. So the complete OOXML of the templates can be rendered using this

Not sure what you mean by this. I am assuming you want to render the templates using the React state when document reopens, right? If that is the case, it is not the best way to go through because you will have to update the state first using CustomXMLPart. Better to directly use the latter to do so.

The deletable thing can also be switched with add to document instead of the document.

For now, let's keeping inserting from library component only.

If a user types anything other than the template and then deletes any template, the typed thing will vanish.

I think unlike because the you are supposed to delete 3 things only - content control, the content inside, and the XML node in CustomXMLPart. Anything outside the content control should be unaffected.

Also a state templatesIns can be made to keep track of OOXML of inserted templates. So the complete OOXML of the templates can be rendered using this

Do you mean the devtools is not working?

Yes, the approach sounds good. Proceed by creating a button. Add a JS event listener which will get triggered onclick. Delete the content control and its content inside. Once that's done, finish the process by deleting the XML node in the CustomXMLPart.

K-Kumar-01 commented 3 years ago

Do you mean the devtools is not working?

Yes it keeps on crashing and reloading. There's also a thread in slack where the full information regarding this is available

Yes, the approach sounds good. Proceed by creating a button. Add a JS event listener which will get triggered onclick.

I apologise but I got confused as to follow which approach and which things. :sweat_smile: Could you please once again clarify what is meant by the above. Thanks

algomaster99 commented 3 years ago

Yes it keeps on crashing and reloading.

That's weird. But I hope you figure out. It is tough to find things related to MS Word add-in development.

Could you please once again clarify what is meant by the above.

Sure.

  1. Append the template identifier to the state in document component whenever a contract is added to the document using the library component.
  2. Render this list of template identifiers in the document component. Make sure you also render a button corresponding to each list item (template identifiers).
  3. Whenever you click the delete button, the following should take place
    1. Delete the content control for the entire contract/clause including its content.
    2. Delete the XML node containing the template identifier.
    3. Delete the template identifier from the state you are maintaining.

When you reinitialise the document, make sure you populate the state maintained in your document component using the template identifier stored in CustomXML.

K-Kumar-01 commented 3 years ago

@algomaster99 Thanks for the explanation. Also I will try to see if I am able to find anything to solve the debugging issue.

algomaster99 commented 3 years ago

No problem. Let me know if you have any more follow up questions about the implementation.

K-Kumar-01 commented 3 years ago

@algomaster99 I was implementing the code but I am stuck at a particular place.

              if (!identifierExists) {
                setup(ciceroMark, template);
                newXml += `<template xmlns="${templateIdentifier}" />`;
                newXml += '</templates>';
                Office.context.document.customXmlParts.getByNamespaceAsync(CUSTOM_XML_NAMESPACE, res => {
                  if (res.status === Office.AsyncResultStatus.Succeeded) {
                    for (let index=0; index<res.value.length; ++index) {
                      res.value[index].deleteAsync();
                    }
                  }
                });
                Office.context.document.customXmlParts.addAsync(newXml);
              } else {
                console.log('XML NOW', newXml);
                Office.context.document.customXmlParts.getByNamespaceAsync(CUSTOM_XML_NAMESPACE, res => {
                  if (res.status === Office.AsyncResultStatus.Succeeded) {
                    for (let index=0; index<res.value.length; ++index) {
                      res.value[index].deleteAsync();
                    }
                  }
                });
                Office.context.document.customXmlParts.addAsync(newXml);
                toast(
                  {
                    title: 'Duplicate template',
                    description: <p>Template cannot be inserted as it is already present in the document.</p>,
                    type: 'error',
                    time: 5000,
                    animation: 'fly down',
                  },
                );
              }

This code is the part where we insert more than one template. So from what I Understood if the template to be inserted is not present. Then the newXML will include our template identifier. Also, this is the code which removes the old Custom XML and inserts the new XML.

                Office.context.document.customXmlParts.getByNamespaceAsync(CUSTOM_XML_NAMESPACE, res => {
                  if (res.status === Office.AsyncResultStatus.Succeeded) {
                    for (let index=0; index<res.value.length; ++index) {
                      res.value[index].deleteAsync();
                    }
                  }
                });
                Office.context.document.customXmlParts.addAsync(newXml);

Now, if the identifier is already present, then we need to remove the oldXML.

                Office.context.document.customXmlParts.getByNamespaceAsync(CUSTOM_XML_NAMESPACE, res => {
                  if (res.status === Office.AsyncResultStatus.Succeeded) {
                    for (let index=0; index<res.value.length; ++index) {
                      res.value[index].deleteAsync();
                    }
                  }
                });
                Office.context.document.customXmlParts.addAsync(newXml);

As visible, both the conditions when the template is present and when it is not present use the same code. However, in the second case, the custom XML parts are not being added to the document again. I am quite not able to understand the reasoning for this. The removal of the nodes is happening but not the insertion again.

testing.zip I have attached a zip file that contains the OOXML for the different processes made. Let me know if there is any explanation that I have missed.

algomaster99 commented 3 years ago

@K-Kumar-01 I would appreciate if you could submit a WIP pull request of the implementation. It will be easier for me to debug.

K-Kumar-01 commented 3 years ago

@algomaster99 Yeah sure I will submit a draft PR. I will clean the code a bit and then submit it ASAP.

K-Kumar-01 commented 3 years ago

@algomaster99 I have made a WIP PR. Thanks for the guidance in advance :)