Skip to content
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

migrate luci-mod-freifunk to js #54

Merged
merged 5 commits into from
Dec 15, 2021
Merged

Conversation

andibraeu
Copy link
Member

missing or gone:

  • osm in basic settings to select location
  • default routes in public status
  • freifunk widgets in public index

Signed-off-by: Andreas Bräu [email protected]

missing or gone:

* osm in basic settings to select location
* default routes in public status
* freifunk widgets in public index

Signed-off-by: Andreas Bräu <[email protected]>
@andibraeu
Copy link
Member Author

cc: @SvenRoederer @pmelange

@pmelange
Copy link
Member

pmelange commented Nov 3, 2021

* do we still want to show default route information on the status page?

Is this the default route when smartgw in olsrd is disabled? If so, then it would be "nice to have" but definitaley not a "need to have".

* does anyone use those freifunk widgets, that would be included via https://github.com/freifunk/openwrt-packages/blob/master/modules/luci-mod-freifunk/luasrc/view/freifunk/index.htm#L79

The Hedy and Falter firmwares both do not use freifunk-widgets. If other communities use them, I'm not sure. I would say to just leave them out.

* the OSM in Basic Settings will follow later

cc: @SvenRoederer @pmelange

@andibraeu
Copy link
Member Author

The default routes thing look like that in the old view:

image

@pmelange
Copy link
Member

pmelange commented Nov 3, 2021

I don't think it is needed. If I go to Status->Routes I see the following...
Screenshot_2021-11-03_13-31-48

@@ -60,41 +21,6 @@ function index()

-- backend
assign({"mini", "freifunk"}, {"admin", "freifunk"}, _("Freifunk"), 5)
Copy link
Member Author

@andibraeu andibraeu Nov 4, 2021

Choose a reason for hiding this comment

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

@jow- is there something equivalent to assign in the new json-based menu?

Copy link
Contributor

Choose a reason for hiding this comment

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

@andibraeu - theoretically you just need to add different menu items invoking the same action (view, template, call, etc.)

Copy link
Member Author

@andibraeu andibraeu Nov 21, 2021

Choose a reason for hiding this comment

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

But it is not possible to "inherit" a complete submenu structure from another package like in

	page = assign({"freifunk", "olsr"}, {"admin", "status", "olsr"}, _("OLSR"), 30)
	page.setuser = false
	page.setgroup = false
	page.acl_depends = {}

?

@andibraeu andibraeu marked this pull request as ready for review November 7, 2021 22:31
@andibraeu
Copy link
Member Author

It's done so far.
All the pages are migrated (as far it was possible). This should also solve some issues here.

However, we do have some leftovers in modules/luci-mod-freifunk/luasrc/controller/freifunk/freifunk.lua. We would need to know how to rebuild the zeroes testdownload. And the second one is about a pure JSON output. Maybe @jow- can help out or knows someone who could help us.

There are also some links to other modules, e.g. to show the OLSR menu in the unauthenticated freifunk module.

@andibraeu
Copy link
Member Author

This PR should fix #23, #33 and #44

@SvenRoederer @pmelange

@pmelange
Copy link
Member

as I am not active in the repo, and falter firmware doesn't use it, then I would prefer if @SvenRoederer merged this

@SvenRoederer
Copy link
Contributor

Thanks for this rework.

  • widgets: I'm also not aware of anyone who is using this, but my view is not complete. Maybe @XDjackieXD can also comment
  • routes: I also see it as "nice to have" rather than "needed"
  • OSM selection of location: done by acdb7b0, correct?

I'll give it a short test on some device, but prefer merging and fix possible issues / add missing features later

@andibraeu
Copy link
Member Author

  • OSM selection of location: done by acdb7b0, correct?

yes, correct.

I'll give it a short test on some device, but prefer merging and fix possible issues / add missing features later

👍

@XDjackieXD
Copy link
Contributor

  • widgets: I'm also not aware of anyone who is using this, but my view is not complete. Maybe @XDjackieXD can also comment

I didn't even know they existed ^^'
The few nodes that currently use OpenWRT in the FunkFeuer Vienna network only really care about publicly viewable OLSR (1 & 2) information and nickname + e-mail address of the owner (I'm currently working on bringing people back to OpenWRT but for this I'll have to write a setup wizard for FunkFeuer).

Are there any examples what could be done or has been done using the widgets?

@andibraeu
Copy link
Member Author

I guess these are some examples for widgets: https://github.com/freifunk/openwrt-packages/tree/master/applications/luci-app-freifunk-widgets/luasrc/view/freifunk/widgets (I wasn't aware of those widgets, too)

But I'm not sure if we can use them in the shiny new js world as is. I would say not to support them anymore until someone who needs them will reimplement the widgets.

@andibraeu
Copy link
Member Author

@SvenRoederer have you had a chance to test?

@SvenRoederer
Copy link
Contributor

just gave it a short test with recent OpenWrt-snapshot. Seen some small issues when some data-fields are not filled, map-loading and others. But that are minor things, which should not block merging as this will fix the more critical issues (mentioned above)

One comment regarding #44 and dropping the permissions (#44 (comment)): Looks like the new code is not doing this explicitly like the old one, but as we now use the RPCd-acls it seems acceptable to do not.

I did not test OLSR or statistics, as they are not included in my test-build.
But any issues found, can be fixed separately.

@andibraeu
Copy link
Member Author

Access control is handled here: modules/luci-mod-freifunk/root/usr/share/rpcd/acl.d/luci-mod-freifunk-unauthenticated.json

OLSR and Statistics have not been part of this package, but the menu items. That's why there's still an old part is here

* they are not supported anymore after migrating luci-mod-freifunk
* there seems nobody using them

Signed-off-by: Sven Roederer <[email protected]>
@SvenRoederer
Copy link
Contributor

just pushed 7eb0703 to remove the widgets, that are now not supported anymore.
Any concerns or let's merge this PR.

@andibraeu
Copy link
Member Author

no concerns from me

@SvenRoederer SvenRoederer merged commit c75a3b9 into master Dec 15, 2021
@SvenRoederer SvenRoederer deleted the luci-mod-freifunk-to-js branch December 15, 2021 00:08
andibraeu pushed a commit that referenced this pull request Dec 20, 2021
* migrate luci-mod-freifunk to to new JavaScript based design introduced by LuCI in OpenWrt-19.07
* drops the Freifunk-widgets, as the infrastructure is not ported and they seem not to be used at all
andibraeu pushed a commit that referenced this pull request Dec 20, 2021
* migrate luci-mod-freifunk to to new JavaScript based design introduced by LuCI in OpenWrt-19.07
* drops the Freifunk-widgets, as the infrastructure is not ported and they seem not to be used at all

Signed-off-by: Andreas Bräu <[email protected]>
andibraeu pushed a commit that referenced this pull request Dec 20, 2021
* migrate luci-mod-freifunk to to new JavaScript based design introduced by LuCI in OpenWrt-19.07
* drops the Freifunk-widgets, as the infrastructure is not ported and they seem not to be used at all

Signed-off-by: Andreas Bräu <[email protected]>
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.

5 participants