search

MushroomObserver / mushroom-observer

A website for sharing observations of mushrooms.
https://mushroomobserver.org
MIT License
77 stars 25 forks source link

Fix MISSING REFLECTIONS #2083

Closed mo-nathan closed 3 months ago

mo-nathan commented 3 months ago

I think this is what is now wanted. I guess this makes sense. Seems a bit like unnecessary overhead to me.

coveralls commented 3 months ago

Coverage Status

coverage: 94.401%. remained the same when pulling 12be78f7e84a7c91e663eea363a6edc26fd230ab on njw-missing-reflections into dfa9ac66dac2e48134d911e0b50aae281c73cdb0 on main.

pellaea commented 3 months ago

Actually, check_for_broken_references looks for belongs_to reflections. For example, field slips must belong to a project and a user? (Sorry, I've been out doing fieldwork and haven't been able to keep up with your recent work on field slips!)

At some point in the relatively recent past Nimmo added all of the missing associations for all of the other models. He can probably provide a better defense of the policy. But I think they are pretty lazy these days, and if they aren't used, they incur very little cost.

But if you would prefer, feel free to alter check_for_broken_references. I think all we'd need to do is add these two associations to the reflections hash around lines 26-40, since they won't be picked up automatically by searching for belongs_to relationships. My intent here was to make it automatically pick up new associations without requiring us to remember to do anything.

On Sun, Mar 31, 2024 at 4:53 PM Nathan Wilson @.***> wrote:

I think this is what is now wanted. I guess this makes sense. Seems a bit like unnecessary overhead to me.

You can view, comment on, or merge this pull request online at:

https://github.com/MushroomObserver/mushroom-observer/pull/2083 Commit Summary

File Changes

(3 files https://github.com/MushroomObserver/mushroom-observer/pull/2083/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/MushroomObserver/mushroom-observer/pull/2083, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYTNNKRSTDVPKJ2EYBOXSLY3BZWBAVCNFSM6AAAAABFQWITS6VHI2DSMVQWIX3LMV43ASLTON2WKOZSGIYTOMJXHA2TOMI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

mo-nathan commented 3 months ago

It already had the belongs_to relationships. I added the corresponding has_manys (although nothing uses them), but that didn't fix the issue. I finally got them to go away by adding the rows to script/check_for_broken_references. I don't understand the purpose here well enough to have more opinion about it.

mo-nathan commented 3 months ago

@JoeCohen Thanks for pointing that case out. I think I've fixed it up so it will work better in that case. As you suggest I would want the field slip to continue to work.