Skip to content

[16.0][IMP] base_geoengine: Avoid error seen clicking on RecordPanel#454

Open
anusriNPS wants to merge 1 commit into
OCA:16.0from
PyTech-SRL:16.0-enhance-geoengine
Open

[16.0][IMP] base_geoengine: Avoid error seen clicking on RecordPanel#454
anusriNPS wants to merge 1 commit into
OCA:16.0from
PyTech-SRL:16.0-enhance-geoengine

Conversation

@anusriNPS
Copy link
Copy Markdown
Contributor

@anusriNPS anusriNPS commented Apr 21, 2026

Depends on #417. When a record in record panel does not contain latitude and
longitude details, it fails with error "feature is null". So, avoiding this error and notifying the user to update required details to view the record.

Below is the error seen without latitude and longitude details:

image

@anusriNPS
Copy link
Copy Markdown
Contributor Author

pre-commit fails which requires #442 to be merged which helps to update template

@anusriNPS anusriNPS force-pushed the 16.0-enhance-geoengine branch from bab27b0 to da82969 Compare April 23, 2026 10:08
Copy link
Copy Markdown

@quirino95 quirino95 left a comment

Choose a reason for hiding this comment

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

Code and functional review: LGTM!

Copy link
Copy Markdown
Contributor

@HekkiMelody HekkiMelody left a comment

Choose a reason for hiding this comment

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

Code review. Please specify in the first message that this PR depends on #417

Comment on lines +577 to +580
this.env._t(
"Please update latitude and longitude details for: " +
record.data.display_name
),
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.

chore: This will not work for translations, only literal strings can be translated

https://www.odoo.com/documentation/18.0/developer/howtos/translations.html

Can you test formatting it like this to see if it's exported properly in the .pot file?

https://github.com/OCA/OCB/blob/18.0/addons/base_import/static/src/import_action/import_action.js#L227-L229

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated pot files for newly introduced notification as part of the PR and translations are working as expected

image

onDisplayPopupRecord(record) {
const popup = this.getPopup();
const feature = this.vectorSource.getFeatureById(record.resId);
if (feature === null) {
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.

question: this if can probably replace the other if at line 588

for example

if (feature) {
    ...
    return false;
}
this.mountGeoengineRecord({
...

What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agree, the new if block is sufficient and hence removed if block which checks for feature is not undefined.

   When a record in record panel does not contain latitude and
longitude details, it fails with error "feature is null". Avoiding
this error and notifying the user to update required details to
view the record.
@anusriNPS anusriNPS force-pushed the 16.0-enhance-geoengine branch from 005a243 to 082ec56 Compare May 29, 2026 13:37
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.

4 participants