Skip to content

Refactor Edges search interfaces: remove $.extend usage and extract reusable URL utility#2562

Open
varadekd wants to merge 83 commits into
developfrom
feature/4185_code_refactor
Open

Refactor Edges search interfaces: remove $.extend usage and extract reusable URL utility#2562
varadekd wants to merge 83 commits into
developfrom
feature/4185_code_refactor

Conversation

@varadekd

@varadekd varadekd commented Mar 9, 2026

Copy link
Copy Markdown
Contributor
  • Issue:

Refactor Edges search interfaces: remove $.extend usage and extract reusable URL utility

This change performs a small refactor of the Edges search interface code under /portality/static/js/edges/ to reduce duplicated infrastructure logic. The previous $.extend(true, doaj, {...}) pattern used to attach namespaces has been replaced with direct namespace assignments, as deep merge behaviour was not required. In addition, repeated URL construction logic (deriving scheme and host to build the search URL) has been extracted into a shared utility (doaj.edgeUtil.url.build). This keeps the search files focused on defining search behaviour while moving common infrastructure code into a reusable location. No functional behaviour has been changed; this update only improves code clarity and maintainability.

This PR...

  • has scripts to run
  • has migrations to run
  • adds new infrastructure
  • changes the CI pipeline
  • affects the public site
  • affects the editorial area
  • affects the publisher area
  • affects the monitoring

Developer Checklist

Developers should review and confirm each of these items before requesting review

  • Code meets acceptance criteria from issue
  • Unit tests are written and all pass
  • User Test Scripts (if required) are written and have been run through
  • Project's coding standards are met
    • No deprecated methods are used
    • No magic strings/numbers - all strings are in constants or messages files
    • ES queries are wrapped in a Query object rather than inlined in the code
    • Where possible our common library functions have been used (e.g. dates manipulated via dates)
    • Cleaned up commented out code, etc
    • Urls are constructed with url_for not hard-coded
  • Code documentation and related non-code documentation has all been updated
  • Migation has been created and tested
  • There is a recent merge from develop

Reviewer Checklist

Reviewers should review and confirm each of these items before approval
If there are multiple reviewers, this section should be duplicated for each reviewer

  • Code meets acceptance criteria from issue
  • Unit tests are written and all pass
  • User Test Scripts (if required) are written and have been run through
  • Project's coding standards are met
    • No deprecated methods are used
    • No magic strings/numbers - all strings are in constants or messages files
    • ES queries are wrapped in a Query object rather than inlined in the code
    • Where possible our common library functions have been used (e.g. dates manipulated via dates)
    • Cleaned up commented out code, etc
    • Urls are constructed with url_for not hard-coded
  • Code documentation and related non-code documentation has all been updated
  • Migation has been created and tested
  • There is a recent merge from develop

Testing

List user test scripts that need to be run

List any non-unit test scripts that need to be run by reviewers

Deployment

What deployment considerations are there? (delete any sections you don't need)

Configuration changes

What configuration changes are included in this PR, and do we need to set specific values for production

Scripts

What scripts need to be run from the PR (e.g. if this is a report generating feature), and when (once, regularly, etc).

Migrations

What migrations need to be run to deploy this

Monitoring

What additional monitoring is required of the application as a result of this feature

New Infrastructure

What new infrastructure does this PR require (e.g. new services that need to run on the back-end).

Continuous Integration

What CI changes are required for this

@richard-jones richard-jones left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A couple of changes to get this to a mergeable state:

  1. Could you just put the url.js functions into the main doaj.js library? That's used everywhere, and I could see these functions being useful in other places.

  2. The search object searches don't seem to be wired in. Could you add an admin view route (in view/admin.py) for each one and serve the page with the appropriate search initialised. They should also be added to the "System Objects" navigation entry in the admin area.

Could you also make sure to run over all the interfaces and do a side-by-side comparison with the live interfaces to make sure we haven't lost any functionality by accident. Particularly we should focus on the public views.

@usman-cottagelabs maybe this is a good place to bring you in too, to review the functionality? I'll assign you to the issue and add some notes.

usman-cottagelabs commented Apr 23, 2026

Copy link
Copy Markdown

Open Requirement in TestQuality

@usman-cottagelabs usman-cottagelabs changed the title Refactor Edges search interfaces: remove $.extend usage and extract reusable URL utility Results per page dropdown not working as expected Apr 23, 2026

Copy link
Copy Markdown

@usman-cottagelabs

usman-cottagelabs commented Apr 23, 2026

Copy link
Copy Markdown

@varadekd bug found:
Precondition
User Role: admin

Steps to Reproduce:
Step 1
Go to the DOAJ admin applications search at /admin/applications

Step 2
Change the search results dropdown from 10 to 25

Step 3
Attempt to change it back to 10

Expected Result
All the options should always be displayed

Actual Result
The option for 10 disappears

See recording attached.
Screen Recording 2026-04-23 155707.mp4

Note: This works fine on the Prod site

This also isn't working on /search/journals
Screen Recording 2026-04-28 161433.mp4

@usman-cottagelabs usman-cottagelabs changed the title Results per page dropdown not working as expected Refactor Edges search interfaces: remove $.extend usage and extract reusable URL utility Apr 27, 2026
@usman-cottagelabs

Copy link
Copy Markdown

@varadekd bug found:

Precondition

User Role: admin

Steps to Reproduce:

Step 1

Go to the DOAJ admin application search at /admin/applications

Step 2

Filter for Rejected

Step 3

Click View Application button within a rejected Journal

Expected Result

In the Notes section on the RHS, There should a note attached to the record which indicates that the record was rejected

Actual Result

No note indicting this

Recording

Screen Recording 2026-04-27 110800.mp4

Note

I have checked Prod site, and it does show this note:

@usman-cottagelabs

Copy link
Copy Markdown

@varadekd a high priority bug found:

Precondition

User Role: admin

Steps to Reproduce:

Step 1

Go to the DOAJ admin users search at /account

Expected Result

Data should show, and search/filter functionality etc

Actual Result

Nothing shows

Screenshot

@varadekd

varadekd commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

UPDATE:
Not a bug. Was a data issue that caused this most likely.

@varadekd bug found:

Precondition

User Role: admin

Steps to Reproduce:

Step 1

Go to the DOAJ admin application search at /admin/applications

Step 2

Filter for Rejected

Step 3

Click View Application button within a rejected Journal

Expected Result

In the Notes section on the RHS, There should a note attached to the record which indicates that the record was rejected

Actual Result

No note indicting this

Recording

Screen Recording 2026-04-27 110800.mp4

Note

I have checked Prod site, and it does show this note:

Hi @usman-cottagelabs
I will verify this on https://latest.doaj.cottagelabs.com/ since on my local I am able to see this
image

@usman-cottagelabs

Copy link
Copy Markdown

Thanks @varadekd
Let me know when all bugs are fixed, and i can retest them in 1 go.

@varadekd

varadekd commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @varadekd Let me know when all bugs are fixed, and i can retest them in 1 go.

@usman-cottagelabs All the valid changes are pushed under the branch feature/4185_code_refactor. You can go ahead with your testing

@usman-cottagelabs

Copy link
Copy Markdown

@varadekd bug found: Precondition User Role: admin

Steps to Reproduce: Step 1 Go to the DOAJ admin applications search at /admin/applications

Step 2 Change the search results dropdown from 10 to 25

Step 3 Attempt to change it back to 10

Expected Result All the options should always be displayed

Actual Result The option for 10 disappears

See recording attached. Screen Recording 2026-04-23 155707.mp4

Note: This works fine on the Prod site

This also isn't working on /search/journals Screen Recording 2026-04-28 161433.mp4​​

Bug fixed

@usman-cottagelabs

Copy link
Copy Markdown

@varadekd a high priority bug found:

Precondition

User Role: admin

Steps to Reproduce:

Step 1

Go to the DOAJ admin users search at /account

Expected Result

Data should show, and search/filter functionality etc

Actual Result

Nothing shows

Screenshot

Bug fixed

@usman-cottagelabs

usman-cottagelabs commented Jun 2, 2026

Copy link
Copy Markdown

Testing complete. @varadekd

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants