Update autocompleteSuccess.php#434
Conversation
In AtoM 2.3, the search was changed "from search?query=Dalhousie&repos=" to "informationobject/browse?topLod=0&query=Dalhousie&repos=". The autocomplete drop down for "all matching descriptions" was not updated to match the changes. If a user selects it, they will be shown encoded json instead of a valid AtoM search page.
|
Hello Margaret! Thank you very much for identifying this bug, and for submitting a PR to resolve it. Our developers have their plates pretty full with client projects at the moment, so apologies in advance if it takes us a little bit to review the fix and merge it. In case you haven't seen them, we have some developer resources on our wiki, which include some information about our coding standards and our code review process: One thing we'll have to ask you to do before we can merge the patch - can you please sign one of our contributor agreements? A link to the downloadable PDF form, and some information on why we require this, can be found at: Thanks again! Regards, Dan Gillean, MAS, MLIS |
sbreker
left a comment
sbreker
left a comment
There was a problem hiding this comment.
Hello! I have completed reviewing your change - code looks good! I will ask if you could re-format your commit message to follow the standards here:
https://wiki.accesstomemory.org/Development/Code_review#Commit_summaries_should_be_concise
The key things:
- first line of commit message should be < 50 chars - (this looks good!)
- first line of commit message should refer to the ticket number:
e.g. Background bulk file import, refs #12345 - second line of commit message should be a blank line
- body of message - each line should wrap at no more than 72 characters.
On your task branch, if you do a 'git commit --amend' you will be able to amend your message. After you save your amended commit message, you can push again, and your Pull Request will be updated here automatically.
Let me know if you have any questions!
Steve
|
Hi Margaret, FYI, I've filed an issue in our tracker relating to this issue here: https://projects.artefactual.com/issues/10295 So you can reference #10295 in the issue title. Thanks in advance! |
In AtoM 2.3, the search was changed "from search?query=Dalhousie&repos=" to "informationobject/browse?topLod=0&query=Dalhousie&repos=". The autocomplete drop down for "all matching descriptions" was not updated to match the changes. If a user selects it, they will be shown encoded json instead of a valid AtoM search page.
|
I am new to updating commit messages - I am not sure if that went through properly as their are 3 commits now... |
|
Hi Margaret Hmm - strange re: the three commits - not sure what happened there. I have gone ahead and merged your change to qa/2.4.x. Code looks good as I said before - thanks for the contribution! You can view your merged patch here: Steve |
|
Merged in 5f83110 |
4 participants
In AtoM 2.3, the search was changed "from search?query=Dalhousie&repos=" to "informationobject/browse?topLod=0&query=Dalhousie&repos=". The autocomplete drop down for "all matching descriptions" was not updated to match the changes. If a user selects it, they will be shown encoded json instead of a valid AtoM search page.