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

Four errors: failure if location contains querystring/hashmark, failure to fetch journal from received doc and more #110

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

Conversation

MayamaTakeshi
Copy link

Hi, while testing redmine_issue_dynamic_edit with latest redmine:

$ git log | head -n 5
commit 109962363437a7f5e9f724059a657f96dbe5de64
Author: Go MAEDA <[email protected]>
Date:   Tue Apr 18 14:45:47 2023 +0000

    Support "my bookmarks" in the search (#38459).

wiithout any other plugins installed I verified that if the tab location contains querystrings (or hashmark) like:

http://192.168.225.201:3000/issues/19?tab=history

the plugin javascript code will crash with this:

Error: Error occured: 
    checkVersion http://192.168.225.201:3000/plugin_assets/redmine_issue_dynamic_edit/javascripts/issue_dynamic_edit.js?1682683385:328
    promise callback*checkVersion http://192.168.225.201:3000/plugin_assets/redmine_issue_dynamic_edit/javascripts/issue_dynamic_edit.js?1682683385:306
    checkVersionInterval http://192.168.225.201:3000/plugin_assets/redmine_issue_dynamic_edit/javascripts/issue_dynamic_edit.js?1682683385:342
    setInterval handler*setCheckVersionInterval http://192.168.225.201:3000/plugin_assets/redmine_issue_dynamic_edit/javascripts/issue_dynamic_edit.js?1682683385:341
    <anonymous> http://192.168.225.201:3000/plugin_assets/redmine_issue_dynamic_edit/javascripts/issue_dynamic_edit.js?1682683385:350

This is happening because the code will try to do a fetch for:

http://192.168.225.201:3000/issues/19?tab=history.json

So I added code to clean up the LOCATION_HREF at the beginning of the script.

Also, no matter if the LOCATION_HREF is clean or not, when handling the response from the server, the code will crash at another spot:

Uncaught TypeError: Node.appendChild: Argument 1 is not an object.
    onreadystatechange http://192.168.225.201:3000/plugin_assets/redmine_issue_dynamic_edit/javascripts/issue_dynamic_edit.js?1682748498:412
    updateIssue http://192.168.225.201:3000/plugin_assets/redmine_issue_dynamic_edit/javascripts/issue_dynamic_edit.js?1682748498:377
    sendData http://192.168.225.201:3000/plugin_assets/redmine_issue_dynamic_edit/javascripts/issue_dynamic_edit.js?1682748498:443
    checkVersion http://192.168.225.201:3000/plugin_assets/redmine_issue_dynamic_edit/javascripts/issue_dynamic_edit.js?1682748498:325
    promise callback*checkVersion http://192.168.225.201:3000/plugin_assets/redmine_issue_dynamic_edit/javascripts/issue_dynamic_edit.js?1682748498:306
    sendData http://192.168.225.201:3000/plugin_assets/redmine_issue_dynamic_edit/javascripts/issue_dynamic_edit.js?1682748498:441
    getEditFormHTML http://192.168.225.201:3000/plugin_assets/redmine_issue_dynamic_edit/javascripts/issue_dynamic_edit.js?1682748498:94
issue_dynamic_edit.js:412:53

I added code to solve this too.

@MayamaTakeshi
Copy link
Author

MayamaTakeshi commented Apr 29, 2023

Actually, the error 2 was happening only in case

   Display comments "in reverse chronological order"

was used.
Using "in chronological order" there was no error.
The problem remains as this requires the javascript code to know the order and get/insert the journal in the correct order.
But I found another error that is in case we try to dynamically modify the details of an issue that doesn't have any history entries yet, the javascript code will crash and further changes will not be possible. So for this, I just added code to prevent the crash. So the notes/history will not be updated but dynamic update will continue to be possible.

UPDATE: I have added code to support comments in reverse order.

@AndreySolod
Copy link

Hi, I am check your solution.
I installed clear Kubuntu 22.10 system on the virtual machine, ruby - standard, obtained via apt-get install ruby ruby-dev, database - postgres, downloaded redmine 5.0.5, cloned issue_dynamics_edit to the plugins folder, start redmine in production environment and on the webrick.
Indeed, I did not pay attention earlier, but from the start this plugin does not work - an error occurs on a virtual machine in the yandex browser (crome like):

Uncaught TypeError: Cannot read properties of null (reading 'appendChild')
    at request.onreadystatechange

Ok, try to install nodejs and npm via apt-get.
After restarting Redmine, the error went away and the plugin worked, but after I added a comment, some problems really appeared:
image
It's really strange - I was sure that this error occurs due to conflicts with other plugins, and by itself it will work.
The firefox browser is friendlier - at least it wrote the following error (although there was already another error there):
<Provider> does not support changing store on the fly. It is most likely that you see this error because you updated to Redux 2.x and React Redux 2.x which no longer hot reload reducers automatically. See https://github.com/reactjs/react-redux/releases/tag/v2.0.0 for the migration instructions.
... But everything suddenly worked, it was only necessary to execute the command RAILS_ENV=production bundle exec rake redmine:plugins:assets
Ok, let's try to install your version.
All work correctly, even some old errors were fixed when updating the task, but one small detail remained: when updating the task, a comment is added to the currently open history tab (notes), although it should not be added there - it should be added to the History and State Change tabs:
image
Well, and besides, the "Update" button adds a new record about updating the task, although it should not (a record about changing the title appeared 3 times)
The incorrect entry disappears if you start switching between tabs.

…ld not be visible if current tab is tab-notes
@MayamaTakeshi
Copy link
Author

Also, when adding journal for issue details changes, if the current tab is tab-notes, the journal should be added but not be made visible. I have added another commit to resolve this.

@MayamaTakeshi
Copy link
Author

Regarding the problem of absent history, I have added code to create it if necessary.

@MayamaTakeshi
Copy link
Author

So to summarize what this PR contemplates:

  1. javascript code will crash and dynamic update will stop working if url contains querystring or hashmarks like
    http://192.168.225.201:3000/issues/38?tab=properties
    http://192.168.225.201:3000/issues/38#note-3
    SOLVED
  2. javascript code crashes and dynamic update will stop working if user is configured with "Display comments in reverse chronological order"
    SOLVED
  3. when adding journals in history, the javascript code doesn't check which is the tab currently enabled. For example, if it is tab-notes, a dynamic change of issue properties should add the journal but not make the entry visible.
    SOLVED
  4. if there is no history entries yet, javascript code will crash and dynamic update will stop working
    SOLVED: will create appropriate div history if absent

@MayamaTakeshi MayamaTakeshi changed the title Two errors: failure if location contains querystring/hashmark, failure to fetch journal from received doc Four errors: failure if location contains querystring/hashmark, failure to fetch journal from received doc and more Oct 20, 2023
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