Skip to content

Conversation

@phl0
Copy link
Contributor

@phl0 phl0 commented Jul 12, 2023

As we are updating several infos like IOTA, state, county I guess we should also update grid from LoTW confirmations. The code introduced in 614452b obviously has a problem here.

I logged a QSO with @int2001 and put his grid wrong (e.g. JO31OO). LoTW confirmation comes in and contains the correct grid (JO30xx). The code would not update the QSO's grid because it is longer than 4 chars but not longer than 6 chars which leads to an SQL statement that does not give a match. So no update on the grid here.

Imho the LoTW confirmation is the most specific grid info here so we should just update the QSO with grid info from LoTW instead of trying to determine which is more specific. Also this would prevent updates if a user logged a wrong but more specific grid ...

@magicbug
Copy link
Owner

Yeah updating grid seems a sane thing todo, catches errors does this handle VUCC @phl0 or just the main gridsquare field.

@phl0
Copy link
Contributor Author

phl0 commented Jul 12, 2023

I think the old code didn't handle VUCC_GRIDS .... I might need to have a look at that. But harder to find sample data for confirmations :)

@magicbug
Copy link
Owner

Ha, i dont think it did.. OK shall we mege this?

@phl0
Copy link
Contributor Author

phl0 commented Jul 12, 2023

Yes please and I need to look at VUCC_GRIDS separately.

@magicbug magicbug marked this pull request as ready for review July 12, 2023 14:11
@magicbug magicbug merged commit 10bb0be into magicbug:dev Jul 12, 2023
@phl0 phl0 deleted the lotwImportGrid branch July 12, 2023 14:14
@phl0
Copy link
Contributor Author

phl0 commented Jul 12, 2023

Yeah updating grid seems a sane thing todo, catches errors does this handle VUCC @phl0 or just the main gridsquare field.

See #2278. There is the implementation for VUCC_GRIDS.

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