-
Notifications
You must be signed in to change notification settings - Fork 18
Pm map fix #1481
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
Pm map fix #1481
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe documentation and test cases for polygon-related XPath functions were updated to use latitude-longitude coordinate order instead of longitude-latitude. Changes were made to parameter descriptions, example usages, and test data to reflect this new convention. No implementation or public API changes were made. Changes
Sequence Diagram(s)sequenceDiagram
participant TestSuite as XPathEvalTest
participant XPathFunc as XPath Function (closest-point-on-polygon / is-point-inside-polygon)
participant Polygon as Polygon Data
TestSuite->>XPathFunc: Call function with "lat lon" ordered coordinates
XPathFunc->>Polygon: Parse coordinates (lat, lon)
Polygon-->>XPathFunc: Polygon structure
XPathFunc-->>TestSuite: Result (using lat-lon order)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/main/java/org/javarosa/xpath/expr/XPathClosestPointOnPolygonFunc.java (1)
21-23:⚠️ Potential issueFix typo in signature –
polygon_cord→polygon_coords- * closest-point-on-polygon(point_coord,polygon_cord) + * closest-point-on-polygon(point_coord, polygon_coords)
🧹 Nitpick comments (6)
src/main/java/org/javarosa/xpath/expr/XPathIsPointInsidePolygonFunc.java (2)
26-28: Clarify parameter wording for consistencyThe second bullet still says “lat/lon pairs”, re-introducing the slash you just removed elsewhere.
To avoid another round of confusion, stick to the exact wording “lat lon pairs”.- <li><code>polygon_coords</code>: A space-separated string of lat/lon pairs … + <li><code>polygon_coords</code>: A space-separated string of lat lon pairs …
36-39: Trim the trailing space in the example inputThe example string has a stray space after the first coordinate which may be copied into forms verbatim and break parsing.
- is-point-inside-polygon('27.174957 78.041309 ','27.174957 … + is-point-inside-polygon('27.174957 78.041309','27.174957 …src/main/java/org/javarosa/xpath/expr/XPathClosestPointOnPolygonFunc.java (2)
37-40: Minor: add space after comma in example callFor readability and to match the actual syntax used in tests.
-closest-point-on-polygon('27.176 78.041','27.174957 … +closest-point-on-polygon('27.176 78.041', '27.174957 …
55-57: Use consistent wording – “lat lon” instead of “Lat Lng”The rest of the doc now standardises on “lat lon”.
- * Returns the point on polygon closest to the geopoint, in "Lat Lng", … + * Returns the point on polygon closest to the geopoint, in "lat lon" format, …src/test/java/org/javarosa/xpath/test/XPathEvalTest.java (2)
660-670: Duplicate test – remove one copyThe block appears twice with identical inputs and expected output, providing no extra coverage and lengthening the suite.
- testEval( - "closest-point-on-polygon('27.175 91.043','27.174957 91.041309 27.174884 91.042574 27.175493 91.042661 27.175569 91.041383')", - null, null, "27.175057319999997 91.04259876");
681-683: Second duplicate of the same assertionSame call/expectation already present above; safe to delete.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/org/javarosa/xpath/expr/XPathClosestPointOnPolygonFunc.java(2 hunks)src/main/java/org/javarosa/xpath/expr/XPathIsPointInsidePolygonFunc.java(2 hunks)src/test/java/org/javarosa/xpath/test/XPathEvalTest.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
| * <li><code>point_coord</code>: A single point as "lat lon eg('27.175 78.043')"</li> | ||
| * <li><code>polygon_coords</code>: A space-separated string of lat/lon pairs (e.g. "'27.174957 78.041309 27 | ||
| * .174884 78.042574 27.175493 78.042661 27.175569 78.041383'")</li> | ||
| * </ul> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Re-format Javadoc bullets – example breaks the list and HTML
- The example is inserted right inside the quoted string, making the sentence hard to read.
- A newline splits the literal string and leaves the
<li>open, which causes malformed Javadoc.
Suggested minimal patch:
- <li><code>point_coord</code>: A single point as "lat lon eg('27.175 78.043')"</li>
- <li><code>polygon_coords</code>: A space-separated string of lat/lon pairs (e.g. "'27.174957 78.041309
- .174884 78.042574 27.175493 78.042661 27.175569 78.041383'")</li>
+ <li><code>point_coord</code>: A single point in <code>"lat lon"</code> format
+ (e.g. <code>"27.175 78.043"</code>)</li>
+ <li><code>polygon_coords</code>: Space-separated <code>lat lon</code> pairs,
+ e.g. <code>"27.174957 78.041309 27.174884 78.042574 27.175493 78.042661 27.175569 78.041383"</code></li>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * <li><code>point_coord</code>: A single point as "lat lon eg('27.175 78.043')"</li> | |
| * <li><code>polygon_coords</code>: A space-separated string of lat/lon pairs (e.g. "'27.174957 78.041309 27 | |
| * .174884 78.042574 27.175493 78.042661 27.175569 78.041383'")</li> | |
| * </ul> | |
| * <li><code>point_coord</code>: A single point in <code>"lat lon"</code> format | |
| * (e.g. <code>"27.175 78.043"</code>)</li> | |
| * <li><code>polygon_coords</code>: Space-separated <code>lat lon</code> pairs, | |
| * e.g. <code>"27.174957 78.041309 27.174884 78.042574 27.175493 78.042661 27.175569 78.041383"</code></li> |
🤖 Prompt for AI Agents
In src/main/java/org/javarosa/xpath/expr/XPathClosestPointOnPolygonFunc.java
around lines 26 to 29, the Javadoc list items are malformed because the example
string is broken by a newline and the example is embedded inside the quoted
string, breaking the list structure. Fix this by moving the example outside the
quoted string and ensuring the entire <li> element is on one continuous line
without line breaks, so the list renders correctly and the example is clearly
separated from the string literal.
Product Description
Mp fix from long Lat input to Lat Lng duplicate of the master PR
checked for valid coordinates of lat lng
Technical Summary
Safety Assurance
Safety story
Automated test coverage
QA Plan
Special deploy instructions
Rollback instructions
Review
Duplicate PR
Automatically duplicate this PR as defined in contributing.md.
Summary by CodeRabbit
Documentation
Tests